From 082c44f3556e4e939c31cb66dda261af4f70bea8 Mon Sep 17 00:00:00 2001 From: Jade Ellis Date: Sun, 15 Feb 2026 16:09:38 +0000 Subject: [PATCH] fix: Only sync LDAP admin status when `admin_filter` is configured Closes #1307 --- changelog.d/1307.bugfix.md | 1 + src/api/client/session.rs | 16 ++++++++++------ src/service/users/mod.rs | 15 ++++++++++----- 3 files changed, 21 insertions(+), 11 deletions(-) create mode 100644 changelog.d/1307.bugfix.md diff --git a/changelog.d/1307.bugfix.md b/changelog.d/1307.bugfix.md new file mode 100644 index 00000000..76077e5f --- /dev/null +++ b/changelog.d/1307.bugfix.md @@ -0,0 +1 @@ +LDAP-enabled servers will no longer have all admins demoted when LDAP-controlled admins are not configured. Contributed by @Jade diff --git a/src/api/client/session.rs b/src/api/client/session.rs index 11ea06b7..f9afc266 100644 --- a/src/api/client/session.rs +++ b/src/api/client/session.rs @@ -107,7 +107,7 @@ pub(super) async fn ldap_login( ) -> Result { let (user_dn, is_ldap_admin) = match services.config.ldap.bind_dn.as_ref() { | Some(bind_dn) if bind_dn.contains("{username}") => - (bind_dn.replace("{username}", lowercased_user_id.localpart()), false), + (bind_dn.replace("{username}", lowercased_user_id.localpart()), None), | _ => { debug!("Searching user in LDAP"); @@ -144,12 +144,16 @@ pub(super) async fn ldap_login( .await?; } - let is_conduwuit_admin = services.admin.user_is_admin(lowercased_user_id).await; + // Only sync admin status if LDAP can actually determine it. + // None means LDAP cannot determine admin status (manual config required). + if let Some(is_ldap_admin) = is_ldap_admin { + let is_conduwuit_admin = services.admin.user_is_admin(lowercased_user_id).await; - if is_ldap_admin && !is_conduwuit_admin { - Box::pin(services.admin.make_user_admin(lowercased_user_id)).await?; - } else if !is_ldap_admin && is_conduwuit_admin { - Box::pin(services.admin.revoke_admin(lowercased_user_id)).await?; + if is_ldap_admin && !is_conduwuit_admin { + Box::pin(services.admin.make_user_admin(lowercased_user_id)).await?; + } else if !is_ldap_admin && is_conduwuit_admin { + Box::pin(services.admin.revoke_admin(lowercased_user_id)).await?; + } } Ok(user_id) diff --git a/src/service/users/mod.rs b/src/service/users/mod.rs index 8599cd68..11510bbb 100644 --- a/src/service/users/mod.rs +++ b/src/service/users/mod.rs @@ -1269,12 +1269,12 @@ impl Service { } #[cfg(not(feature = "ldap"))] - pub async fn search_ldap(&self, _user_id: &UserId) -> Result> { + pub async fn search_ldap(&self, _user_id: &UserId) -> Result)>> { Err!(FeatureDisabled("ldap")) } #[cfg(feature = "ldap")] - pub async fn search_ldap(&self, user_id: &UserId) -> Result> { + pub async fn search_ldap(&self, user_id: &UserId) -> Result)>> { let localpart = user_id.localpart().to_owned(); let lowercased_localpart = localpart.to_lowercase(); @@ -1318,7 +1318,7 @@ impl Service { .inspect(|(entries, result)| trace!(?entries, ?result, "LDAP Search")) .map_err(|e| err!(Ldap(error!(?attr, ?user_filter, "LDAP search error: {e}"))))?; - let mut dns: HashMap = entries + let mut dns: HashMap> = entries .into_iter() .filter_map(|entry| { let search_entry = SearchEntry::construct(entry); @@ -1329,11 +1329,16 @@ impl Service { .into_iter() .chain(search_entry.attrs.get(&config.name_attribute)) .any(|ids| ids.contains(&localpart) || ids.contains(&lowercased_localpart)) - .then_some((search_entry.dn, false)) + .then_some((search_entry.dn, None)) }) .collect(); if !config.admin_filter.is_empty() { + // Update all existing entries to Some(false) since we can now determine admin + // status + for admin_status in dns.values_mut() { + *admin_status = Some(false); + } let admin_base_dn = if config.admin_base_dn.is_empty() { &config.base_dn } else { @@ -1362,7 +1367,7 @@ impl Service { .into_iter() .chain(search_entry.attrs.get(&config.name_attribute)) .any(|ids| ids.contains(&localpart) || ids.contains(&lowercased_localpart)) - .then_some((search_entry.dn, true)) + .then_some((search_entry.dn, Some(true))) })); }