From ac58df3bbb1d0fd5212dde6521c9a76b192407bd Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Thu, 2 Jul 2026 13:24:21 +0200 Subject: [PATCH 1/5] ldap dry run 1 --- .../src/enterprise/ldap/client.rs | 6 +- .../src/enterprise/ldap/model.rs | 21 +- .../defguard_core/src/enterprise/ldap/sync.rs | 166 +++++++++++++-- .../src/enterprise/ldap/test_client.rs | 10 +- .../src/enterprise/ldap/tests.rs | 189 +++++++++++++++++- crates/defguard_core/src/handlers/settings.rs | 60 +++++- crates/defguard_core/src/lib.rs | 11 +- .../tests/integration/api/settings.rs | 55 ++++- web/messages/en/modal.json | 15 +- web/messages/en/settings.json | 2 +- .../SettingsLdapPage/SettingsLdapPage.tsx | 83 +++++--- .../modals/LdapDryRunModal/DryRunTable.tsx | 123 ++++++++++++ .../LdapDryRunModal/LdapDryRunModal.tsx | 51 +++++ .../modals/LdapDryRunModal/style.scss | 40 ++++ web/src/shared/api/api.ts | 3 + web/src/shared/api/types.ts | 15 ++ 16 files changed, 798 insertions(+), 52 deletions(-) create mode 100644 web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/DryRunTable.tsx create mode 100644 web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/LdapDryRunModal.tsx create mode 100644 web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/style.scss diff --git a/crates/defguard_core/src/enterprise/ldap/client.rs b/crates/defguard_core/src/enterprise/ldap/client.rs index c475efd6ee..793dac82e7 100644 --- a/crates/defguard_core/src/enterprise/ldap/client.rs +++ b/crates/defguard_core/src/enterprise/ldap/client.rs @@ -29,7 +29,11 @@ fn try_construct_entry(entry: ResultEntry) -> Option { impl LDAPConnection { pub async fn create() -> Result { - let settings = Settings::get_current_settings(); + Self::create_with_settings(Settings::get_current_settings()).await + } + + /// Establishes an LDAP connection using the provided settings + pub async fn create_with_settings(settings: Settings) -> Result { let config = LDAPConfig::try_from(settings.clone())?; let url = settings.ldap_url.ok_or(LdapError::MissingSettings( "LDAP URL is required for LDAP configuration to work".to_owned(), diff --git a/crates/defguard_core/src/enterprise/ldap/model.rs b/crates/defguard_core/src/enterprise/ldap/model.rs index 05732d2e17..e07418dd3d 100644 --- a/crates/defguard_core/src/enterprise/ldap/model.rs +++ b/crates/defguard_core/src/enterprise/ldap/model.rs @@ -297,7 +297,26 @@ where { let settings = Settings::get_current_settings(); let sync_account_status = settings.ldap_uses_ad && settings.ldap_sync_account_status; - let sync_groups = settings.ldap_sync_groups; + ldap_sync_allowed_for_user_scoped( + user, + executor, + sync_account_status, + &settings.ldap_sync_groups, + ) + .await +} + +/// Same as [`ldap_sync_allowed_for_user`] but with the scoping settings passed explicitly. +/// Needed by flows running with settings that differ from the saved ones (LDAP dry run). +pub(crate) async fn ldap_sync_allowed_for_user_scoped<'e, E>( + user: &User, + executor: E, + sync_account_status: bool, + sync_groups: &[String], +) -> sqlx::Result +where + E: PgExecutor<'e>, +{ let my_groups = user.member_of(executor).await?; Ok( (sync_groups.is_empty() || my_groups.iter().any(|g| sync_groups.contains(&g.name))) diff --git a/crates/defguard_core/src/enterprise/ldap/sync.rs b/crates/defguard_core/src/enterprise/ldap/sync.rs index a68ad1a59a..8715efc37a 100644 --- a/crates/defguard_core/src/enterprise/ldap/sync.rs +++ b/crates/defguard_core/src/enterprise/ldap/sync.rs @@ -78,6 +78,7 @@ use defguard_common::db::{ settings::{LdapSyncStatus, update_current_settings}, }, }; +use serde::Serialize; use sqlx::{PgConnection, PgPool}; use tokio::sync::{broadcast::Sender, mpsc::UnboundedSender}; @@ -86,8 +87,8 @@ use crate::{ enrollment_management::try_send_ldap_enrollment_invite, enterprise::{ ldap::model::{ - get_users_without_ldap_path, ldap_sync_allowed_for_user, update_from_ldap_user, - user_from_searchentry, + get_users_without_ldap_path, ldap_sync_allowed_for_user, + ldap_sync_allowed_for_user_scoped, update_from_ldap_user, user_from_searchentry, }, license::get_cached_license, limits::{get_counts, update_counts}, @@ -166,6 +167,74 @@ pub(super) struct UserSyncChanges { pub add_ldap: Vec>, } +#[derive(Debug, Clone, Copy, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum LdapDryRunAction { + Add, + Remove, +} + +#[derive(Debug, Serialize)] +pub struct LdapDryRunUser { + pub username: String, + pub email: String, + pub first_name: String, + pub last_name: String, + pub action: LdapDryRunAction, +} + +/// Preview of the user changes a full sync would make, split by the system that would be +/// modified. Built from [`UserSyncChanges`]. +#[derive(Debug, Serialize)] +pub struct LdapDryRunResult { + pub defguard: Vec, + pub ldap: Vec, +} + +fn dry_run_user(user: &User, action: LdapDryRunAction) -> LdapDryRunUser { + LdapDryRunUser { + username: user.username.clone(), + email: user.email.clone(), + first_name: user.first_name.clone(), + last_name: user.last_name.clone(), + action, + } +} + +impl From for LdapDryRunResult { + fn from(changes: UserSyncChanges) -> Self { + let mut defguard = Vec::new(); + defguard.extend( + changes + .add_defguard + .iter() + .map(|u| dry_run_user(u, LdapDryRunAction::Add)), + ); + defguard.extend( + changes + .delete_defguard + .iter() + .map(|u| dry_run_user(u, LdapDryRunAction::Remove)), + ); + + let mut ldap = Vec::new(); + ldap.extend( + changes + .add_ldap + .iter() + .map(|u| dry_run_user(u, LdapDryRunAction::Add)), + ); + ldap.extend( + changes + .delete_ldap + .iter() + .map(|u| dry_run_user(u, LdapDryRunAction::Remove)), + ); + + Self { defguard, ldap } + } +} + /// Computes what users should be added/deleted and where pub(super) fn compute_user_sync_changes( all_ldap_users: &mut Vec, @@ -758,17 +827,7 @@ impl super::LDAPConnection { sync_group_members.extend(members); } - let mut all_ldap_users = self.get_all_users().await?; - let mut all_defguard_users = User::all(pool).await?; - - // Filter out users that should be ignored from sync - let mut filtered_users = Vec::new(); - for user in all_defguard_users { - if ldap_sync_allowed_for_user(&user, pool).await? { - filtered_users.push(user); - } - } - all_defguard_users = filtered_users; + let (mut all_ldap_users, mut all_defguard_users) = self.get_sync_users(pool).await?; let ldap_usernames = all_ldap_users .iter() @@ -833,6 +892,87 @@ impl super::LDAPConnection { Ok(()) } + /// Fetches all LDAP users alongside the Defguard users that are allowed to participate in + /// sync, filtering out the ones that should be ignored. + async fn get_sync_users( + &mut self, + pool: &PgPool, + ) -> Result<(Vec, Vec>), LdapError> { + let all_ldap_users = self.get_all_users().await?; + let all_defguard_users = User::all(pool).await?; + + let sync_account_status = self.config.ldap_uses_ad && self.config.ldap_sync_account_status; + let mut filtered_users = Vec::new(); + for user in all_defguard_users { + if ldap_sync_allowed_for_user_scoped( + &user, + pool, + sync_account_status, + &self.config.ldap_sync_groups, + ) + .await? + { + filtered_users.push(user); + } + } + + Ok((all_ldap_users, filtered_users)) + } + + /// Computes the user additions/removals a full sync would perform, without applying any + /// of them. + pub async fn dry_run( + &mut self, + pool: &PgPool, + authority: Authority, + ) -> Result { + debug!("Performing LDAP dry run with authority: {authority:?}"); + + let (mut all_ldap_users, mut all_defguard_users) = self.get_sync_users(pool).await?; + + // Mirror `fix_missing_user_path()` in memory. + let ldap_paths_by_username: HashMap<&str, (&str, Option<&str>)> = all_ldap_users + .iter() + .map(|u| { + ( + u.username.as_str(), + (u.ldap_rdn_value(), u.ldap_user_path.as_deref()), + ) + }) + .collect(); + for defguard_user in &mut all_defguard_users { + if defguard_user.ldap_user_path.is_some() { + continue; + } + if let Some((ldap_rdn, ldap_path)) = + ldap_paths_by_username.get(defguard_user.username.as_str()) + { + if defguard_user.ldap_rdn_value() == *ldap_rdn { + defguard_user.ldap_user_path = ldap_path.map(str::to_owned); + } + } + } + + let mut user_changes = compute_user_sync_changes( + &mut all_ldap_users, + &mut all_defguard_users, + authority, + &self.config, + ); + + let existing_usernames = User::all(pool) + .await? + .into_iter() + .map(|user| user.username) + .collect::>(); + user_changes + .add_defguard + .retain(|user| !existing_usernames.contains(&user.username)); + + debug!("LDAP dry run completed"); + Ok(LdapDryRunResult::from(user_changes)) + } + async fn apply_user_group_sync_changes( &mut self, pool: &PgPool, diff --git a/crates/defguard_core/src/enterprise/ldap/test_client.rs b/crates/defguard_core/src/enterprise/ldap/test_client.rs index 5c05add2c3..b61e607488 100644 --- a/crates/defguard_core/src/enterprise/ldap/test_client.rs +++ b/crates/defguard_core/src/enterprise/ldap/test_client.rs @@ -4,7 +4,7 @@ use std::{ vec::Vec, }; -use defguard_common::db::models::{User, group::Group}; +use defguard_common::db::models::{Settings, User, group::Group}; use ldap3::{Mod, SearchEntry}; use super::{LDAPConfig, LDAPConnection, error::LdapError}; @@ -265,6 +265,14 @@ impl LDAPConnection { }) } + pub async fn create_with_settings(settings: Settings) -> Result { + Ok(Self { + config: LDAPConfig::try_from(settings)?, + url: String::new(), + test_client: TestClient::default(), + }) + } + pub(super) async fn search_users( &mut self, filter: &str, diff --git a/crates/defguard_core/src/enterprise/ldap/tests.rs b/crates/defguard_core/src/enterprise/ldap/tests.rs index 6cd49eee3c..57d99e331d 100644 --- a/crates/defguard_core/src/enterprise/ldap/tests.rs +++ b/crates/defguard_core/src/enterprise/ldap/tests.rs @@ -20,7 +20,7 @@ use tokio::sync::{ use super::{ model::{extract_rdn_value, get_users_without_ldap_path, user_from_searchentry}, sync::{ - Authority, compute_group_sync_changes, compute_user_sync_changes, + Authority, LdapDryRunAction, compute_group_sync_changes, compute_user_sync_changes, extract_intersecting_users, is_ldap_desynced, set_ldap_sync_status, }, test_client::{LdapEvent, group_to_test_attrs, user_to_test_attrs}, @@ -1841,6 +1841,193 @@ async fn test_fix_missing_user_path(_: PgPoolOptions, options: PgConnectOptions) } } +/// A dry run must preview the additions/removals a full sync would make while writing nothing +/// to LDAP or the Defguard database. This guards the hard "no import" requirement of the LDAP +/// setup preview. +#[sqlx::test] +async fn test_ldap_dry_run_previews_changes_without_writing( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = setup_pool(options).await; + let _ = initialize_current_settings(&pool).await; + set_test_license_business(); + + let mut ldap_conn = super::LDAPConnection::create().await.unwrap(); + let config = ldap_conn.config.clone(); + + // Present only in Defguard: a full sync with LDAP authority would remove this user. + make_test_user("defguard_only", Some("defguard_only".to_owned()), None) + .save(&pool) + .await + .unwrap(); + + // Present only in LDAP: a full sync with LDAP authority would import this user. + let ldap_only = make_test_user("ldap_only", Some("ldap_only".to_owned()), None); + ldap_conn + .test_client_mut() + .add_test_user(&ldap_only, &config); + + let before = defguard_sync_snapshot(&pool).await; + ldap_conn.test_client_mut().clear_events(); + + let result = ldap_conn.dry_run(&pool, Authority::LDAP).await.unwrap(); + + let added: Vec<_> = result + .defguard + .iter() + .filter(|u| matches!(u.action, LdapDryRunAction::Add)) + .map(|u| u.username.as_str()) + .collect(); + let removed: Vec<_> = result + .defguard + .iter() + .filter(|u| matches!(u.action, LdapDryRunAction::Remove)) + .map(|u| u.username.as_str()) + .collect(); + assert_eq!(added, vec!["ldap_only"]); + assert_eq!(removed, vec!["defguard_only"]); + + // The dry run must not touch LDAP or the Defguard database. + assert!( + ldap_conn.test_client.get_events().is_empty(), + "dry run emitted LDAP operations: {:?}", + ldap_conn.test_client.get_events() + ); + assert_eq!( + before, + defguard_sync_snapshot(&pool).await, + "dry run mutated Defguard state" + ); +} + +/// A Defguard user that exists in LDAP but has no stored LDAP path must not be previewed as +/// both removed and re-added. A real full sync backfills the missing path first (so the DNs +/// match and the user is treated as unchanged); the dry run replicates that in memory. +#[sqlx::test] +async fn test_ldap_dry_run_does_not_double_list_user_with_missing_path( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = setup_pool(options).await; + let _ = initialize_current_settings(&pool).await; + set_test_license_business(); + + let mut ldap_conn = super::LDAPConnection::create().await.unwrap(); + let config = ldap_conn.config.clone(); + + // Locally-created user with no LDAP path yet, same RDN as the LDAP entry. + make_test_user("shared_user", Some("shared_user".to_owned()), None) + .save(&pool) + .await + .unwrap(); + + // Same user in LDAP, living in an OU (so its DN differs until the path is reconciled). + let mut ldap_user = make_test_user("shared_user", Some("shared_user".to_owned()), None); + ldap_user.ldap_user_path = Some("ou=people,dc=example,dc=com".to_owned()); + ldap_conn + .test_client_mut() + .add_test_user(&ldap_user, &config); + + let result = ldap_conn.dry_run(&pool, Authority::LDAP).await.unwrap(); + + let mentions: Vec<_> = result + .defguard + .iter() + .chain(result.ldap.iter()) + .filter(|u| u.username == "shared_user") + .collect(); + assert!( + mentions.is_empty(), + "user with a missing path was listed as a change: {mentions:?}" + ); +} + +/// A disabled Defguard user that also exists in LDAP is outside the sync scope, so the change +/// computation sees them as missing from Defguard. A real sync skips such additions because +/// the username already exists in the database; the dry run must not preview them as "to be +/// added" either. +#[sqlx::test] +async fn test_ldap_dry_run_skips_disabled_users_existing_in_defguard( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = setup_pool(options).await; + let _ = initialize_current_settings(&pool).await; + set_test_license_business(); + + let mut ldap_conn = super::LDAPConnection::create().await.unwrap(); + let config = ldap_conn.config.clone(); + + let mut disabled_user = make_test_user("disabled_user", Some("disabled_user".to_owned()), None); + disabled_user.is_active = false; + disabled_user.save(&pool).await.unwrap(); + + let ldap_user = make_test_user("disabled_user", Some("disabled_user".to_owned()), None); + ldap_conn + .test_client_mut() + .add_test_user(&ldap_user, &config); + + let result = ldap_conn.dry_run(&pool, Authority::LDAP).await.unwrap(); + + let mentions: Vec<_> = result + .defguard + .iter() + .chain(result.ldap.iter()) + .filter(|u| u.username == "disabled_user") + .collect(); + assert!( + mentions.is_empty(), + "disabled user already present in Defguard was listed as a change: {mentions:?}" + ); +} + +/// The dry run previews not yet saved settings, so user scoping must follow the connection's +/// config (built from the submitted form values) instead of the globally saved settings. +/// Here the saved settings have no sync group restriction, while the submitted config limits +/// the sync to one group: only members of that group may appear in the preview. +#[sqlx::test] +async fn test_ldap_dry_run_scopes_users_by_submitted_settings( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = setup_pool(options).await; + let _ = initialize_current_settings(&pool).await; + set_test_license_business(); + + let mut ldap_conn = super::LDAPConnection::create().await.unwrap(); + // Mirrors a dry run request whose form values restrict the sync to one group. + ldap_conn.config.ldap_sync_groups = vec!["ldap_sync_group".to_owned()]; + + let sync_group = Group::new("ldap_sync_group").save(&pool).await.unwrap(); + + // Both users exist only in Defguard, so with LDAP authority any in-scope user is + // previewed as removed. + let synced_user = make_test_user("synced_user", Some("synced_user".to_owned()), None) + .save(&pool) + .await + .unwrap(); + synced_user.add_to_group(&pool, &sync_group).await.unwrap(); + make_test_user("outside_user", Some("outside_user".to_owned()), None) + .save(&pool) + .await + .unwrap(); + + let result = ldap_conn.dry_run(&pool, Authority::LDAP).await.unwrap(); + + let removed: Vec<_> = result + .defguard + .iter() + .filter(|u| matches!(u.action, LdapDryRunAction::Remove)) + .map(|u| u.username.as_str()) + .collect(); + assert_eq!( + removed, + vec!["synced_user"], + "preview must scope users by the submitted sync groups, not the saved settings" + ); +} + #[sqlx::test] async fn test_sync_users_with_empty_paths_and_nested_ous( _: PgPoolOptions, diff --git a/crates/defguard_core/src/handlers/settings.rs b/crates/defguard_core/src/handlers/settings.rs index 374c13f7c0..2dc23b2b03 100644 --- a/crates/defguard_core/src/handlers/settings.rs +++ b/crates/defguard_core/src/handlers/settings.rs @@ -17,7 +17,11 @@ use super::{ApiResponse, ApiResult}; use crate::{ AppState, auth::{AdminRole, SessionInfo}, - enterprise::{handlers::LicenseInfo, ldap::LDAPConnection, license::update_cached_license}, + enterprise::{ + handlers::LicenseInfo, + ldap::{LDAPConnection, sync::Authority}, + license::update_cached_license, + }, error::WebError, events::{ApiEvent, ApiEventType, ApiRequestContext}, }; @@ -179,3 +183,57 @@ pub(crate) async fn test_ldap_settings(_admin: AdminRole, _license: LicenseInfo) } } } + +/// Tests the LDAP connection using the provided (not yet saved) settings. +pub(crate) async fn test_submitted_ldap_settings( + _admin: AdminRole, + _license: LicenseInfo, + Json(settings): Json, +) -> ApiResult { + debug!("Testing LDAP connection with provided settings"); + match LDAPConnection::create_with_settings(settings).await { + Ok(_) => { + debug!("LDAP connected successfully"); + Ok(ApiResponse::with_status(StatusCode::OK)) + } + Err(err) => { + debug!("LDAP connection rejected: {err}"); + Ok(ApiResponse::with_status(StatusCode::BAD_REQUEST)) + } + } +} + +/// Previews the user changes a full LDAP sync would make using the provided (not yet saved) +/// settings. This is strictly read-only: nothing is imported, removed or persisted. +pub(crate) async fn ldap_dry_run( + _admin: AdminRole, + _license: LicenseInfo, + State(appstate): State, + Json(settings): Json, +) -> ApiResult { + debug!("Performing LDAP dry run with provided settings"); + let authority = if settings.ldap_is_authoritative { + Authority::LDAP + } else { + Authority::Defguard + }; + + let mut connection = match LDAPConnection::create_with_settings(settings).await { + Ok(connection) => connection, + Err(err) => { + debug!("LDAP dry run connection rejected: {err}"); + return Ok(ApiResponse::with_status(StatusCode::BAD_REQUEST)); + } + }; + + match connection.dry_run(&appstate.pool, authority).await { + Ok(result) => { + debug!("LDAP dry run completed successfully"); + Ok(ApiResponse::json(result, StatusCode::OK)) + } + Err(err) => { + debug!("LDAP dry run failed: {err}"); + Ok(ApiResponse::with_status(StatusCode::BAD_REQUEST)) + } + } +} diff --git a/crates/defguard_core/src/lib.rs b/crates/defguard_core/src/lib.rs index 9e31821425..6b78ecdd6a 100644 --- a/crates/defguard_core/src/lib.rs +++ b/crates/defguard_core/src/lib.rs @@ -161,8 +161,9 @@ use crate::{ proxy::{delete_proxy, proxy_details, proxy_list, update_proxy}, resource_display::get_locations_display, settings::{ - get_settings, get_settings_essentials, patch_settings, set_default_branding, - test_ldap_settings, update_settings, + get_settings, get_settings_essentials, ldap_dry_run, patch_settings, + set_default_branding, test_ldap_settings, test_submitted_ldap_settings, + update_settings, }, ssh_authorized_keys::get_authorized_keys, static_ips::{ @@ -416,7 +417,11 @@ pub fn build_webapp( .post(change_enabled), ) // ldap - .route("/ldap/test", get(test_ldap_settings)) + .route( + "/ldap/test", + get(test_ldap_settings).post(test_submitted_ldap_settings), + ) + .route("/ldap/dry_run", post(ldap_dry_run)) // activity log .route("/activity_log", get(get_activity_log_events)) // Proxy routes diff --git a/crates/defguard_core/tests/integration/api/settings.rs b/crates/defguard_core/tests/integration/api/settings.rs index 6541768d89..37357af459 100644 --- a/crates/defguard_core/tests/integration/api/settings.rs +++ b/crates/defguard_core/tests/integration/api/settings.rs @@ -1,6 +1,11 @@ -use defguard_common::db::models::{ - Settings, - settings::{SettingsPatch, update_current_settings}, +use std::str::FromStr; + +use defguard_common::{ + db::models::{ + Settings, + settings::{SettingsPatch, update_current_settings}, + }, + secret::SecretStringWrapper, }; use defguard_core::handlers::Auth; use reqwest::StatusCode; @@ -193,6 +198,50 @@ async fn test_ldap_settings_validation(_: PgPoolOptions, options: PgConnectOptio ); } +#[sqlx::test] +async fn test_ldap_connection_test_with_submitted_settings( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = setup_pool(options).await; + let (client, _client_state) = make_test_client(pool).await; + + let auth = Auth::new("admin", "pass123"); + let response = client.post("/api/v1/auth").json(&auth).send().await; + assert_eq!(response.status(), StatusCode::OK); + + let response = client.get("/api/v1/settings").send().await; + assert_eq!(response.status(), StatusCode::OK); + let saved_settings: Settings = response.json().await; + + let mut submitted = saved_settings.clone(); + submitted.ldap_url = Some("ldap://127.0.0.1:1".to_owned()); + submitted.ldap_bind_username = Some("cn=admin,dc=example,dc=com".to_owned()); + submitted.ldap_bind_password = Some(SecretStringWrapper::from_str("secret").unwrap()); + submitted.ldap_username_attr = Some("uid".to_owned()); + submitted.ldap_user_search_base = Some("ou=users,dc=example,dc=com".to_owned()); + submitted.ldap_user_obj_class = Some("inetOrgPerson".to_owned()); + submitted.ldap_member_attr = Some("memberUid".to_owned()); + submitted.ldap_groupname_attr = Some("cn".to_owned()); + submitted.ldap_group_obj_class = Some("posixGroup".to_owned()); + submitted.ldap_group_member_attr = Some("memberUid".to_owned()); + submitted.ldap_group_search_base = Some("ou=groups,dc=example,dc=com".to_owned()); + let response = client.post("/api/v1/ldap/test").json(&submitted).send().await; + assert_eq!( + response.status(), + StatusCode::BAD_REQUEST, + "connection test against an unreachable server should return 400" + ); + + let response = client.get("/api/v1/settings").send().await; + assert_eq!(response.status(), StatusCode::OK); + let settings_after: Settings = response.json().await; + assert_eq!( + settings_after, saved_settings, + "the connection test must not save the submitted settings" + ); +} + #[sqlx::test] async fn test_ldap_remote_enrollment_validation(_: PgPoolOptions, options: PgConnectOptions) { let pool = setup_pool(options).await; diff --git a/web/messages/en/modal.json b/web/messages/en/modal.json index 23448affcf..9c80625e54 100644 --- a/web/messages/en/modal.json +++ b/web/messages/en/modal.json @@ -258,5 +258,18 @@ "modal_app_update_go_to_release": "Go to release page", "modal_app_update_dismiss": "Dismiss", "modal_app_update_snackbar_message": "New version {version} is available", - "modal_app_update_snackbar_action": "What's new" + "modal_app_update_snackbar_action": "What's new", + "modal_ldap_dry_run_title": "Test synchronization results", + "modal_ldap_dry_run_description": "The results below show the outcome of the connection check using the current synchronization settings. These changes will be applied to the system if the configuration is saved and activated.", + "modal_ldap_dry_run_tab_defguard": "Defguard", + "modal_ldap_dry_run_tab_ldap": "LDAP", + "modal_ldap_dry_run_search_placeholder": "Search", + "modal_ldap_dry_run_col_username": "Username", + "modal_ldap_dry_run_col_email": "Email", + "modal_ldap_dry_run_col_status": "Status", + "modal_ldap_dry_run_status_added": "Will be added", + "modal_ldap_dry_run_status_removed": "Will be removed", + "modal_ldap_dry_run_empty_title": "No changes detected", + "modal_ldap_dry_run_empty_subtitle": "Based on the current LDAP configuration, no changes will be applied to users in this group.", + "modal_ldap_dry_run_search_empty_subtitle": "Please try another search request." } diff --git a/web/messages/en/settings.json b/web/messages/en/settings.json index 855c5b0f1c..c8fd73d285 100644 --- a/web/messages/en/settings.json +++ b/web/messages/en/settings.json @@ -297,7 +297,7 @@ "settings_ldap_helper_sync_interval": "", "settings_ldap_toggle_enable_integration": "Enable LDAP integration", "settings_ldap_button_test_connection": "Test connection", - "settings_ldap_test_connection_tooltip": "To test connection please fill the form and save the changes first.", + "settings_ldap_test_connection_tooltip": "To test the synchronization, please fill in all required fields first.", "settings_ldap_section_remote_enrollment_title": "Secure remote enrollment", "settings_ldap_section_remote_enrollment_description": "Control how Defguard synchronizes users with LDAP — disable sync, import users from LDAP, or keep both systems updated automatically.", "settings_ldap_label_remote_enrollment_enabled": "Enable Secure Remote Enrollment", diff --git a/web/src/pages/settings/SettingsLdapPage/SettingsLdapPage.tsx b/web/src/pages/settings/SettingsLdapPage/SettingsLdapPage.tsx index bf2328183d..b486957082 100644 --- a/web/src/pages/settings/SettingsLdapPage/SettingsLdapPage.tsx +++ b/web/src/pages/settings/SettingsLdapPage/SettingsLdapPage.tsx @@ -11,11 +11,12 @@ import { SettingsLayout } from '../../../shared/components/SettingsLayout/Settin import './style.scss'; import { useStore } from '@tanstack/react-form'; import { useMutation, useQuery, useSuspenseQuery } from '@tanstack/react-query'; -import { Suspense, useMemo } from 'react'; +import { Suspense, useMemo, useState } from 'react'; import Skeleton from 'react-loading-skeleton'; import z from 'zod'; import { m } from '../../../paraglide/messages'; import api from '../../../shared/api/api'; +import type { LdapDryRunResult, Settings } from '../../../shared/api/types'; import { businessBadgeProps } from '../../../shared/components/badges/BusinessBadge'; import { Controls } from '../../../shared/components/Controls/Controls'; import { DescriptionBlock } from '../../../shared/components/DescriptionBlock/DescriptionBlock'; @@ -43,6 +44,7 @@ import { getSettingsQueryOptions, } from '../../../shared/query'; import { canUseBusinessFeature } from '../../../shared/utils/license'; +import { LdapDryRunModal } from './modals/LdapDryRunModal/LdapDryRunModal'; const breadcrumbsLinks = [ ; +const csvToArray = (value: string | null): string[] => + value + ? value + .split(',') + .map((item) => item.trim()) + .filter(Boolean) + : []; + const PageForm = () => { - const isAppLdapEnabled = useApp((s) => s.appInfo.ldap_info.enabled); const smtpEnabled = useApp((s) => s.appInfo.smtp_enabled); const { data: licenseInfo } = useSuspenseQuery(getLicenseInfoQueryOptions); const { data: settings } = useSuspenseQuery(getSettingsQueryOptions); @@ -181,10 +190,14 @@ const PageForm = () => { }, }); - const { mutate: handleLdapTest, isPending: testInProgress } = useMutation({ - mutationFn: api.settings.getLdapConnectionStatus, - onSuccess: () => { - Snackbar.default(m.settings_ldap_test_success()); + const [dryRunResult, setDryRunResult] = useState(null); + const [dryRunModalOpen, setDryRunModalOpen] = useState(false); + + const { mutate: handleLdapDryRun, isPending: dryRunInProgress } = useMutation({ + mutationFn: (data: Settings) => api.settings.ldapDryRun(data), + onSuccess: (res) => { + setDryRunResult(res.data); + setDryRunModalOpen(true); }, onError: (e) => { Snackbar.error(m.settings_ldap_test_failed()); @@ -192,6 +205,18 @@ const PageForm = () => { }, }); + const { mutate: handleLdapConnectionTest, isPending: connectionTestInProgress } = + useMutation({ + mutationFn: (data: Settings) => api.settings.testLdapSettings(data), + onSuccess: () => { + Snackbar.default(m.settings_ldap_test_success()); + }, + onError: (e) => { + Snackbar.error(m.settings_ldap_test_failed()); + console.error(e); + }, + }); + const form = useAppForm({ defaultValues, validationLogic: formChangeLogic, @@ -208,18 +233,10 @@ const PageForm = () => { await mutateAsync({ ...value, - ldap_user_auxiliary_obj_classes: value.ldap_user_auxiliary_obj_classes - ? value.ldap_user_auxiliary_obj_classes - .split(',') - .map((item) => item.trim()) - .filter(Boolean) - : [], - ldap_sync_groups: value.ldap_sync_groups - ? value.ldap_sync_groups - .split(',') - .map((item) => item.trim()) - .filter(Boolean) - : [], + ldap_user_auxiliary_obj_classes: csvToArray( + value.ldap_user_auxiliary_obj_classes, + ), + ldap_sync_groups: csvToArray(value.ldap_sync_groups), }); formApi.reset(value); }, @@ -607,10 +624,7 @@ const PageForm = () => { {({ isDefaultValue, isSubmitting }) => ( <>
@@ -621,13 +635,25 @@ const PageForm = () => { iconLeft={IconKind.Refresh} disabled={ isSubmitting || - !isDefaultValue || - !isAppLdapEnabled || + !requiredFieldsFilled || !canUseBusinessLicenseCheck } - loading={testInProgress} + loading={dryRunInProgress || connectionTestInProgress} onClick={() => { - handleLdapTest(); + const values = form.state.values; + const submitted = { + ...settings, + ...values, + ldap_user_auxiliary_obj_classes: csvToArray( + values.ldap_user_auxiliary_obj_classes, + ), + ldap_sync_groups: csvToArray(values.ldap_sync_groups), + } as Settings; + if (values.ldap_sync_enabled) { + handleLdapDryRun(submitted); + } else { + handleLdapConnectionTest(submitted); + } }} />
@@ -648,6 +674,11 @@ const PageForm = () => { + setDryRunModalOpen(false)} + /> ); }; diff --git a/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/DryRunTable.tsx b/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/DryRunTable.tsx new file mode 100644 index 0000000000..eaf1f7583e --- /dev/null +++ b/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/DryRunTable.tsx @@ -0,0 +1,123 @@ +import { + createColumnHelper, + getCoreRowModel, + getSortedRowModel, + useReactTable, +} from '@tanstack/react-table'; +import { useMemo, useState } from 'react'; +import { m } from '../../../../../paraglide/messages'; +import type { LdapDryRunUser } from '../../../../../shared/api/types'; +import { Badge } from '../../../../../shared/defguard-ui/components/Badge/Badge'; +import { BadgeVariant } from '../../../../../shared/defguard-ui/components/Badge/types'; +import { EmptyState } from '../../../../../shared/defguard-ui/components/EmptyState/EmptyState'; +import { Search } from '../../../../../shared/defguard-ui/components/Search/Search'; +import { SizedBox } from '../../../../../shared/defguard-ui/components/SizedBox/SizedBox'; +import { TableBody } from '../../../../../shared/defguard-ui/components/table/TableBody/TableBody'; +import { TableCell } from '../../../../../shared/defguard-ui/components/table/TableCell/TableCell'; +import { ThemeSpacing } from '../../../../../shared/defguard-ui/types'; + +const columnHelper = createColumnHelper(); + +const textCell = (info: { getValue: () => string }) => ( + + {info.getValue()} + +); + +export const DryRunTable = ({ data }: { data: LdapDryRunUser[] }) => { + const [search, setSearch] = useState(''); + + const columns = useMemo( + () => [ + columnHelper.accessor('username', { + header: m.modal_ldap_dry_run_col_username(), + enableSorting: true, + sortingFn: 'text', + size: 160, + minSize: 160, + cell: textCell, + }), + columnHelper.accessor('email', { + header: m.modal_ldap_dry_run_col_email(), + enableSorting: true, + sortingFn: 'text', + size: 260, + minSize: 260, + cell: textCell, + }), + columnHelper.accessor('action', { + header: m.modal_ldap_dry_run_col_status(), + enableSorting: true, + sortingFn: 'text', + size: 160, + minSize: 120, + meta: { flex: true }, + cell: (info) => { + const added = info.getValue() === 'add'; + return ( + + + + ); + }, + }), + ], + [], + ); + + const filtered = useMemo(() => { + const query = search.trim().toLowerCase(); + if (!query) return data; + return data.filter( + (user) => + user.username.toLowerCase().includes(query) || + user.email.toLowerCase().includes(query), + ); + }, [data, search]); + + const table = useReactTable({ + columns, + data: filtered, + enableRowSelection: false, + columnResizeMode: 'onChange', + getCoreRowModel: getCoreRowModel(), + getSortedRowModel: getSortedRowModel(), + }); + + if (data.length === 0) { + return ( + + ); + } + + return ( + <> + + + + {filtered.length === 0 ? ( + + ) : ( + + )} + + ); +}; diff --git a/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/LdapDryRunModal.tsx b/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/LdapDryRunModal.tsx new file mode 100644 index 0000000000..9eddd09a95 --- /dev/null +++ b/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/LdapDryRunModal.tsx @@ -0,0 +1,51 @@ +import { useState } from 'react'; +import { m } from '../../../../../paraglide/messages'; +import type { LdapDryRunResult } from '../../../../../shared/api/types'; +import { Modal } from '../../../../../shared/defguard-ui/components/Modal/Modal'; +import { Tabs } from '../../../../../shared/defguard-ui/components/Tabs/Tabs'; +import './style.scss'; +import { SizedBox } from '../../../../../shared/defguard-ui/components/SizedBox/SizedBox'; +import { ThemeSpacing } from '../../../../../shared/defguard-ui/types'; +import { DryRunTable } from './DryRunTable'; + +type Props = { + isOpen: boolean; + result: LdapDryRunResult | null; + onClose: () => void; +}; + +type DryRunTab = 'defguard' | 'ldap'; + +export const LdapDryRunModal = ({ isOpen, result, onClose }: Props) => { + const [tab, setTab] = useState('defguard'); + + const activeData = (tab === 'defguard' ? result?.defguard : result?.ldap) ?? []; + + return ( + +

{m.modal_ldap_dry_run_description()}

+ + setTab('defguard'), + }, + { + title: m.modal_ldap_dry_run_tab_ldap(), + active: tab === 'ldap', + onClick: () => setTab('ldap'), + }, + ]} + /> + + + ); +}; diff --git a/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/style.scss b/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/style.scss new file mode 100644 index 0000000000..c6e16c60cb --- /dev/null +++ b/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/style.scss @@ -0,0 +1,40 @@ +.ldap-dry-run-modal { + height: 720px; + display: flex; + flex-direction: column; + + .description { + color: var(--fg-muted); + font: var(--t-body-sm-400); + } + + .table { + padding: 0; + border-bottom: 1px solid var(--border-disabled); + border-bottom-left-radius: var(--radius-md); + border-bottom-right-radius: var(--radius-md); + overflow: hidden; + + // the container border above replaces it; both would render doubled + .table-row-container.last { + border-bottom: 0; + } + + // per design the overflowing table has no visible scrollbar + .table-scroll { + scrollbar-width: none; + } + } + + & > .modal-content { + display: flex; + flex-direction: column; + flex: 1 1 auto; + min-height: 0; + } + + .empty-state { + flex: 1 1 auto; + justify-content: center; + } +} diff --git a/web/src/shared/api/api.ts b/web/src/shared/api/api.ts index cc4724813f..30ea205834 100644 --- a/web/src/shared/api/api.ts +++ b/web/src/shared/api/api.ts @@ -72,6 +72,7 @@ import type { GetInternalSslInfoResponse, GroupInfo, IpValidation, + LdapDryRunResult, LicenseCheckResponse, LicenseInfo, LicenseInfoResponse, @@ -478,6 +479,8 @@ const api = { client.patch('/settings_enterprise', data), getSettingsEssentials: () => client.get('/settings_essentials'), getLdapConnectionStatus: () => client.get(`/ldap/test`), + testLdapSettings: (data: Settings) => client.post('/ldap/test', data), + ldapDryRun: (data: Settings) => client.post('/ldap/dry_run', data), }, openIdProvider: { getOpenIdProvider: () => diff --git a/web/src/shared/api/types.ts b/web/src/shared/api/types.ts index 363f3a7521..b1343a7166 100644 --- a/web/src/shared/api/types.ts +++ b/web/src/shared/api/types.ts @@ -1175,6 +1175,21 @@ export interface SettingsLDAP { ldap_remote_enrollment_send_invite: boolean; } +export type LdapDryRunAction = 'add' | 'remove'; + +export interface LdapDryRunUser { + username: string; + email: string; + first_name: string; + last_name: string; + action: LdapDryRunAction; +} + +export interface LdapDryRunResult { + defguard: LdapDryRunUser[]; + ldap: LdapDryRunUser[]; +} + export interface SettingsOpenID { openid_create_account: boolean; } From 3276208faabe05d48096811cdcae3e95b1b9fde9 Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Thu, 2 Jul 2026 13:24:44 +0200 Subject: [PATCH 2/5] format --- crates/defguard_core/tests/integration/api/settings.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/defguard_core/tests/integration/api/settings.rs b/crates/defguard_core/tests/integration/api/settings.rs index 37357af459..db61f5475e 100644 --- a/crates/defguard_core/tests/integration/api/settings.rs +++ b/crates/defguard_core/tests/integration/api/settings.rs @@ -226,7 +226,11 @@ async fn test_ldap_connection_test_with_submitted_settings( submitted.ldap_group_obj_class = Some("posixGroup".to_owned()); submitted.ldap_group_member_attr = Some("memberUid".to_owned()); submitted.ldap_group_search_base = Some("ou=groups,dc=example,dc=com".to_owned()); - let response = client.post("/api/v1/ldap/test").json(&submitted).send().await; + let response = client + .post("/api/v1/ldap/test") + .json(&submitted) + .send() + .await; assert_eq!( response.status(), StatusCode::BAD_REQUEST, From bb168cc7904c6efb9f431896c1730bce79eaf749 Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Thu, 2 Jul 2026 14:32:13 +0200 Subject: [PATCH 3/5] change table shrinking --- .../SettingsLdapPage/modals/LdapDryRunModal/DryRunTable.tsx | 2 +- .../SettingsLdapPage/modals/LdapDryRunModal/style.scss | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/DryRunTable.tsx b/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/DryRunTable.tsx index eaf1f7583e..50e0513a15 100644 --- a/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/DryRunTable.tsx +++ b/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/DryRunTable.tsx @@ -116,7 +116,7 @@ export const DryRunTable = ({ data }: { data: LdapDryRunUser[] }) => { subtitle={m.modal_ldap_dry_run_search_empty_subtitle()} /> ) : ( - + )} ); diff --git a/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/style.scss b/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/style.scss index c6e16c60cb..369839c4cd 100644 --- a/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/style.scss +++ b/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/style.scss @@ -1,5 +1,5 @@ .ldap-dry-run-modal { - height: 720px; + min-height: 720px; display: flex; flex-direction: column; @@ -30,7 +30,6 @@ display: flex; flex-direction: column; flex: 1 1 auto; - min-height: 0; } .empty-state { From f6f107a24141860eb112aeca4c278b5b5efbe43a Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Thu, 2 Jul 2026 14:59:18 +0200 Subject: [PATCH 4/5] make table flex --- .../SettingsLdapPage/modals/LdapDryRunModal/DryRunTable.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/DryRunTable.tsx b/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/DryRunTable.tsx index 50e0513a15..408f2b0f5f 100644 --- a/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/DryRunTable.tsx +++ b/web/src/pages/settings/SettingsLdapPage/modals/LdapDryRunModal/DryRunTable.tsx @@ -55,7 +55,7 @@ export const DryRunTable = ({ data }: { data: LdapDryRunUser[] }) => { cell: (info) => { const added = info.getValue() === 'add'; return ( - + Date: Fri, 3 Jul 2026 09:09:14 +0200 Subject: [PATCH 5/5] fix linter errors --- crates/defguard_core/src/enterprise/ldap/sync.rs | 5 ++--- web/src/shared/defguard-ui | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/defguard_core/src/enterprise/ldap/sync.rs b/crates/defguard_core/src/enterprise/ldap/sync.rs index 8715efc37a..d4b6158489 100644 --- a/crates/defguard_core/src/enterprise/ldap/sync.rs +++ b/crates/defguard_core/src/enterprise/ldap/sync.rs @@ -946,10 +946,9 @@ impl super::LDAPConnection { } if let Some((ldap_rdn, ldap_path)) = ldap_paths_by_username.get(defguard_user.username.as_str()) + && defguard_user.ldap_rdn_value() == *ldap_rdn { - if defguard_user.ldap_rdn_value() == *ldap_rdn { - defguard_user.ldap_user_path = ldap_path.map(str::to_owned); - } + defguard_user.ldap_user_path = ldap_path.map(str::to_owned); } } diff --git a/web/src/shared/defguard-ui b/web/src/shared/defguard-ui index 1385d3de92..fe79945735 160000 --- a/web/src/shared/defguard-ui +++ b/web/src/shared/defguard-ui @@ -1 +1 @@ -Subproject commit 1385d3de92f89dffe5603d0f25e8515c5f3fb243 +Subproject commit fe799457355647ef99b75484993bd8d3882c0319