Synchronize enrollment pending users from LDAP

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

@@ -307,6 +307,18 @@ impl<I> User<I> {
         false
     }
 
+    /// Like [`Self::is_enrolled`], but also treats LDAP-origin users whose enrollment is
+    /// still pending as eligible for synchronization.
+    ///
+    /// LDAP can create users that are enrollment-pending by default
+    /// (https://github.com/DefGuard/defguard/issues/2967). They already exist in the
+    /// directory, so they must stay in scope for synchronization even before they finish
+    /// enrolling, otherwise their group membership and account status drift out of sync.
+    #[must_use]
+    pub fn is_enrolled_or_ldap_pending(&self) -> bool {
+        self.is_enrolled() || (self.from_ldap && self.enrollment_pending)
+    }
+
     #[must_use]
     pub fn ldap_rdn_value(&self) -> &str {
         if let Some(ldap_rdn) = &self.ldap_rdn {

crates/defguard_core/src/enterprise/ldap/mod.rs

@@ -388,8 +388,10 @@ impl LDAPConnection {
             // An enrolled, in-scope LDAP user who has just been disabled in Defguard. We detect
             // this before `user_sync_allowed` because disabled users are filtered out there, and
             // we'd otherwise miss the active->disabled transition.
-            let user_disabled_in_defguard =
-                user_in_sync_groups && user_exists_in_ldap && user.is_enrolled() && !user.is_active;
+            let user_disabled_in_defguard = user_in_sync_groups
+                && user_exists_in_ldap
+                && user.is_enrolled_or_ldap_pending()
+                && !user.is_active;
 
             if user_disabled_in_defguard {
                 if sync_account_status {
@@ -408,7 +410,7 @@ impl LDAPConnection {
             let user_enabled_in_defguard = sync_account_status
                 && user_in_sync_groups
                 && user_exists_in_ldap
-                && user.is_enrolled()
+                && user.is_enrolled_or_ldap_pending()
                 && user.is_active;
 
             if user_enabled_in_defguard {

crates/defguard_core/src/enterprise/ldap/model.rs

@@ -286,7 +286,7 @@ pub(crate) fn maybe_update_rdn<I>(user: &mut User<I>) {
 /// - he is in a group that is allowed to be synced or no such groups are configured
 /// - he is active (not disabled), unless AD account status sync is enabled, in which case
 ///   disabled users stay in scope so their status can be kept in sync instead of deleting them
-/// - he is enrolled
+/// - he is enrolled, or is an LDAP-origin user whose enrollment is still pending
 pub(crate) async fn ldap_sync_allowed_for_user<'e, E>(
     user: &User<Id>,
     executor: E,
@@ -301,7 +301,7 @@ where
     Ok(
         (sync_groups.is_empty() || my_groups.iter().any(|g| sync_groups.contains(&g.name)))
             && (user.is_active || sync_account_status)
-            && user.is_enrolled(),
+            && user.is_enrolled_or_ldap_pending(),
     )
 }
 

crates/defguard_core/src/enterprise/ldap/sync.rs

@@ -190,7 +190,7 @@ pub(super) fn compute_user_sync_changes(
             match authority {
                 Authority::LDAP => {
                     // Skip inactive/not enrolled users when deleting from LDAP
-                    if user.is_active && user.is_enrolled() {
+                    if user.is_active && user.is_enrolled_or_ldap_pending() {
                         debug!(
                             "User {} is active and enrolled, removing from Defguard",
                             user.username
@@ -205,7 +205,7 @@ pub(super) fn compute_user_sync_changes(
                 }
                 Authority::Defguard => {
                     // Skip inactive users when adding to LDAP
-                    if user.is_active && user.is_enrolled() {
+                    if user.is_active && user.is_enrolled_or_ldap_pending() {
                         debug!(
                             "User {} is active and enrolled, adding to LDAP",
                             user.username

crates/defguard_core/src/enterprise/ldap/tests.rs

@@ -3489,6 +3489,49 @@ async fn test_ldap_sync_allowed_enrolled_via_ldap(_: PgPoolOptions, options: PgC
     assert!(result);
 }
 
+/// LDAP can create users that are enrollment-pending by default. They already exist in the directory,
+/// so they must stay in scope for synchronization even before they finish enrolling.
+#[sqlx::test]
+async fn test_ldap_sync_allowed_ldap_pending_enrollment(
+    _: PgPoolOptions,
+    options: PgConnectOptions,
+) {
+    let pool = setup_pool(options).await;
+    let _ = initialize_current_settings(&pool).await;
+
+    let mut user = make_test_user("testuser", None, None);
+    user.is_active = true;
+    user.password_hash = None;
+    user.openid_sub = None;
+    user.from_ldap = true;
+    user.enrollment_pending = true;
+    let user = user.save(&pool).await.unwrap();
+
+    let result = ldap_sync_allowed_for_user(&user, &pool).await.unwrap();
+    assert!(result);
+}
+
+/// A non-LDAP user with pending enrollment is not yet a real account and must stay out of sync.
+#[sqlx::test]
+async fn test_ldap_sync_disallowed_non_ldap_pending_enrollment(
+    _: PgPoolOptions,
+    options: PgConnectOptions,
+) {
+    let pool = setup_pool(options).await;
+    let _ = initialize_current_settings(&pool).await;
+
+    let mut user = make_test_user("testuser", None, None);
+    user.is_active = true;
+    user.password_hash = None;
+    user.openid_sub = None;
+    user.from_ldap = false;
+    user.enrollment_pending = true;
+    let user = user.save(&pool).await.unwrap();
+
+    let result = ldap_sync_allowed_for_user(&user, &pool).await.unwrap();
+    assert!(!result);
+}
+
 #[sqlx::test]
 async fn test_ldap_sync_allowed_all_conditions_false(_: PgPoolOptions, options: PgConnectOptions) {
     let pool = setup_pool(options).await;

crates/defguard_core/tests/integration/ldap/mod.rs

@@ -7,7 +7,7 @@ use defguard_common::{
     db::{
         models::{
             Settings, User,
-            group::Group,
+            group::{Group, Permission},
             settings::{initialize_current_settings, set_settings},
         },
         setup_pool,
@@ -76,6 +76,37 @@ async fn set_sync_settings(pool: &PgPool, sync_group: &str, authoritative: bool)
     set_settings(Some(settings));
 }
 
+fn enable_secure_enrollment() {
+    let mut settings = Settings::get_current_settings();
+    settings.ldap_remote_enrollment_enabled = true;
+    settings.ldap_remote_enrollment_send_invite = true;
+    settings.smtp_server = Some("smtp.example.com".into());
+    settings.smtp_port = Some(587);
+    settings.smtp_sender = Some("noreply@test.defguard".into());
+    settings.public_proxy_url = "http://proxy.example.com".into();
+    set_settings(Some(settings));
+}
+
+async fn create_active_admin(pool: &PgPool, username: &str) {
+    let admin_group = Group::new("secure_enroll_admins").save(pool).await.unwrap();
+    admin_group
+        .set_permission(pool, Permission::IsAdmin, true)
+        .await
+        .unwrap();
+    let admin = User::new(
+        username,
+        Some("pass123"),
+        "Admin",
+        "User",
+        &format!("{username}@test.defguard"),
+        None,
+    )
+    .save(pool)
+    .await
+    .unwrap();
+    admin.add_to_group(pool, &admin_group).await.unwrap();
+}
+
 fn user_dn<I>(conn: &LDAPConnection, user: &User<I>) -> String {
     format!(
         "{}={},{}",
@@ -1060,3 +1091,188 @@ async fn test_sync_defguard_authority_is_idempotent(_: PgPoolOptions, options: P
     let _ = ldap_conn.delete_user(&ldap_user_second).await;
     let _ = ldap_conn.delete_group("idem_push_grp").await;
 }
+
+/// A user invited through "allow secure enrollment" (`ldap_remote_enrollment_enabled`) is
+/// enrollment-pending after being pulled in, yet must stay within LDAP sync scope.
+///
+/// Before the fix, `is_enrolled()` resolves to `false` on `enrollment_pending`, so such a
+/// user was dropped from sync entirely: subsequent directory changes (here, an attribute edit in
+/// LDAP) never reached Defguard. The whole scenario:
+///   1. Secure enrollment + invite enabled, LDAP authority, one sync group.
+///   2. The user lives only in LDAP and belongs to the sync group.
+///   3. First sync pulls the user into Defguard, marks them `enrollment_pending`, and mirrors the
+///      group membership.
+///   4. The user's surname is changed in LDAP.
+///   5. A second sync must propagate that change to Defguard - which only happens if the
+///      still-pending user is treated as in scope (and the user must not be deleted).
+#[ignore = "requires LDAP server"]
+#[sqlx::test]
+async fn test_sync_keeps_enrollment_pending_ldap_user_in_scope(
+    _: PgPoolOptions,
+    options: PgConnectOptions,
+) {
+    let pool = setup_pool(options).await;
+    let (wg_tx, _wg_rx) = wg_test_channel();
+    set_sync_settings(&pool, "secure_enroll_grp", true).await;
+    enable_secure_enrollment();
+    create_active_admin(&pool, "secure_enroll_admin").await;
+
+    // The sync group must exist in Defguard for the membership to be mirrored.
+    Group::new("secure_enroll_grp").save(&pool).await.unwrap();
+
+    let mut user = User::new(
+        "secure_enroll_user",
+        Some("pass123"),
+        "Secure",
+        "Enroll",
+        "secure_enroll@test.defguard",
+        None,
+    )
+    .save(&pool)
+    .await
+    .unwrap();
+
+    let mut ldap_conn = LDAPConnection::create().await.unwrap();
+    ldap_conn.config.ldap_uses_ad = env::var("LDAP_USES_AD").is_ok();
+    ldap_conn.config.ldap_sync_groups = vec![String::from("secure_enroll_grp")];
+    let _ = ldap_conn.delete_user(&user).await;
+    let _ = ldap_conn.delete_group("secure_enroll_grp").await;
+
+    // Provision the user in LDAP and in the sync group, then drop the Defguard copy so the first
+    // sync pulls them in fresh (as an LDAP-origin user).
+    ldap_conn
+        .add_user(&mut user, Some("pass123"), &pool)
+        .await
+        .unwrap();
+    ldap_conn
+        .add_user_to_group(&user, "secure_enroll_grp")
+        .await
+        .unwrap();
+    user.clone().delete(&pool).await.unwrap();
+
+    // First sync: pull the LDAP-only user into Defguard. Secure enrollment + invite are on and an
+    // active admin exists, so the user is marked enrollment-pending.
+    ldap_conn.sync(&pool, true, &wg_tx).await.unwrap();
+
+    let pulled = User::find_by_username(&pool, "secure_enroll_user")
+        .await
+        .unwrap()
+        .expect("LDAP user should have been pulled into Defguard");
+    assert!(pulled.from_ldap);
+    assert!(
+        pulled.enrollment_pending,
+        "secure enrollment invite should mark the pulled user as enrollment-pending"
+    );
+    assert!(
+        !pulled.is_enrolled(),
+        "an enrollment-pending user is not yet enrolled"
+    );
+    assert!(
+        pulled
+            .member_of_names(&pool)
+            .await
+            .unwrap()
+            .contains(&String::from("secure_enroll_grp")),
+        "first sync should mirror the LDAP group membership"
+    );
+
+    // Change the user's surname in LDAP, keeping them in the sync group.
+    let mut ldap_side = pulled.clone();
+    ldap_side.last_name = String::from("Renamed");
+    ldap_conn
+        .modify_user(&pulled.username, &ldap_side)
+        .await
+        .unwrap();
+
+    // Second sync: the attribute change must reach Defguard. This only happens when the
+    // still-pending user remains in sync scope - the regression this guards against.
+    ldap_conn.sync(&pool, false, &wg_tx).await.unwrap();
+
+    let after = User::find_by_username(&pool, "secure_enroll_user")
+        .await
+        .unwrap()
+        .expect("pending LDAP user must not be deleted while still present in LDAP");
+    assert_eq!(
+        after.last_name, "Renamed",
+        "attribute change for an enrollment-pending LDAP user must propagate to Defguard"
+    );
+    assert!(
+        after
+            .member_of_names(&pool)
+            .await
+            .unwrap()
+            .contains(&String::from("secure_enroll_grp")),
+        "the pending user should still belong to the sync group"
+    );
+    assert!(
+        after.enrollment_pending,
+        "the user should still be enrollment-pending after sync"
+    );
+
+    let _ = ldap_conn.delete_user(&after).await;
+    let _ = ldap_conn.delete_group("secure_enroll_grp").await;
+}
+
+/// The converse of `test_sync_keeps_enrollment_pending_ldap_user_in_scope`: the
+/// enrollment-pending allowance is scoped to LDAP-origin users only. A Defguard-native user
+/// (`from_ldap = false`) with a pending enrollment is not yet a real account and must NOT be
+/// pushed into LDAP, even under Defguard authority. Clearing the pending flag then lets the same
+/// user through, proving it is the flag that holds them back.
+#[ignore = "requires LDAP server"]
+#[sqlx::test]
+async fn test_sync_does_not_push_pending_defguard_user_to_ldap(
+    _: PgPoolOptions,
+    options: PgConnectOptions,
+) {
+    let pool = setup_pool(options).await;
+    let (wg_tx, _wg_rx) = wg_test_channel();
+    // Defguard authority: a full sync pushes in-scope Defguard-only users into LDAP.
+    set_sync_settings(&pool, "no_push_grp", false).await;
+
+    let group = Group::new("no_push_grp").save(&pool).await.unwrap();
+    // The user has a password, so the only thing keeping them out of sync is the pending flag.
+    let mut user = User::new(
+        "no_push_user",
+        Some("pass123"),
+        "NoPush",
+        "User",
+        "no_push@test.defguard",
+        None,
+    )
+    .save(&pool)
+    .await
+    .unwrap();
+    user.enrollment_pending = true;
+    user.save(&pool).await.unwrap();
+    user.add_to_group(&pool, &group).await.unwrap();
+
+    let mut ldap_conn = LDAPConnection::create().await.unwrap();
+    ldap_conn.config.ldap_uses_ad = env::var("LDAP_USES_AD").is_ok();
+    let _ = ldap_conn.delete_user(&user).await;
+    let _ = ldap_conn.delete_group("no_push_grp").await;
+
+    // While pending, the Defguard-native user must not be created in LDAP.
+    ldap_conn.sync(&pool, true, &wg_tx).await.unwrap();
+    assert!(
+        ldap_conn
+            .get_user_by_username("no_push_user")
+            .await
+            .is_err(),
+        "an enrollment-pending Defguard-native user must not be pushed to LDAP"
+    );
+
+    // Clearing the pending flag makes the user enrolled (they have a password), so the next sync
+    // pushes them - confirming the pending flag was the sole reason they were held back.
+    user.enrollment_pending = false;
+    user.save(&pool).await.unwrap();
+
+    ldap_conn.sync(&pool, true, &wg_tx).await.unwrap();
+    let pushed = ldap_conn
+        .get_user_by_username("no_push_user")
+        .await
+        .expect("once no longer pending, the Defguard user must be pushed to LDAP");
+    assert_eq!(pushed.email, "no_push@test.defguard");
+
+    let _ = ldap_conn.delete_user(&pushed).await;
+    let _ = ldap_conn.delete_group("no_push_grp").await;
+}