Fix Directory: Update local groups only when the external directory includes a group attribute

This commit is contained in:
Maurus Decimus
2026-07-12 15:34:04 +02:00
parent e8192d1920
commit 8d54d0b44c
10 changed files with 116 additions and 74 deletions

View File

@@ -17,6 +17,7 @@ If you are upgrading from v0.16.x, replace the binary (or run `docker pull`). If
- JMAP:
- VacationResponse: `isEnabled` reset to false whenever properties are changed.
- Capabilities: Return RFC-3339-conformant UTCDate literals in capabilities: min `0001-01-01T00:00:00Z`, max `9999-12-31T23:59:59Z`.
- Directory: Update local groups only when the external directory includes a group attribute.
- Reject invalid duration values (e.g. `1h30m`).
- Branding: Custom logos for domains do not work.
- Sieve: add `Received` headers to auto-generated messages and detect loops.

View File

@@ -91,26 +91,28 @@ impl Server {
has_changes = true;
}
}
let mut member_group_ids = Vec::with_capacity(account.groups.len());
for email in account.groups {
member_group_ids.push(
self.synchronize_group(directory::Group {
email,
..Default::default()
})
.await
.caused_by(trc::location!())?
.into(),
);
}
if updated_account.member_group_ids.len() != member_group_ids.len()
|| !updated_account
.member_group_ids
.iter()
.all(|id| member_group_ids.contains(id))
{
updated_account.member_group_ids = member_group_ids.into();
has_changes = true;
if let Some(groups) = account.groups {
let mut member_group_ids = Vec::with_capacity(groups.len());
for email in groups {
member_group_ids.push(
self.synchronize_group(directory::Group {
email,
..Default::default()
})
.await
.caused_by(trc::location!())?
.into(),
);
}
if updated_account.member_group_ids.len() != member_group_ids.len()
|| !updated_account
.member_group_ids
.iter()
.all(|id| member_group_ids.contains(id))
{
updated_account.member_group_ids = member_group_ids.into();
has_changes = true;
}
}
if has_changes {
@@ -168,8 +170,8 @@ impl Server {
});
}
}
let mut member_group_ids = Vec::with_capacity(account.groups.len());
for email in account.groups {
let mut member_group_ids = Vec::new();
for email in account.groups.unwrap_or_default() {
member_group_ids.push(
self.synchronize_group(directory::Group {
email,

View File

@@ -120,11 +120,9 @@ impl LdapDirectory {
conn: &mut Ldap,
result: &mut LdapResult,
) -> trc::Result<()> {
if !result.account.groups.is_empty() {
for name in std::mem::take(&mut result.account.groups)
.into_iter()
.filter(|name| name.contains('='))
{
if let Some(group_dns) = result.account.groups.take() {
let mut groups = Vec::new();
for name in group_dns.into_iter().filter(|name| name.contains('=')) {
let (rs, _res) = conn
.search(
&name,
@@ -142,12 +140,13 @@ impl LdapDirectory {
&& let Some(email) =
value.first().map(|s| s.as_str()).and_then(sanitize_email)
{
result.account.groups.push(email);
groups.push(email);
break 'outer;
}
}
}
}
result.account.groups = if groups.is_empty() { None } else { Some(groups) };
} else if let Some(filter) = &self.mappings.filter_member_of {
let filter = filter.build(&result.dn);
let rs = conn
@@ -162,25 +161,31 @@ impl LdapDirectory {
.success()
.map_err(|err| err.into_error().caused_by(trc::location!()))?
.0;
let had_entries = !rs.is_empty();
let mut groups = Vec::new();
for entry in rs {
for (attr, value) in SearchEntry::construct(entry).attrs {
if self.mappings.attr_email.contains(&attr.to_lowercase()) {
result
.account
.groups
.extend(value.into_iter().filter_map(|v| {
sanitize_email(&v).or_else(|| {
trc::event!(
Store(trc::StoreEvent::LdapWarning),
Reason = "Group entry missing valid email attribute",
Details = v
);
None
})
}));
groups.extend(value.into_iter().filter_map(|v| {
sanitize_email(&v).or_else(|| {
trc::event!(
Store(trc::StoreEvent::LdapWarning),
Reason = "Group entry missing valid email attribute",
Details = v
);
None
})
}));
}
}
}
result.account.groups = if had_entries && groups.is_empty() {
None
} else {
Some(groups)
};
} else {
result.account.groups = Some(Vec::new());
}
Ok(())
@@ -253,7 +258,7 @@ impl LdapMappings {
account.description = Some(desc);
}
} else if self.attr_groups.contains(&attr) {
account.groups.extend(value);
account.groups.get_or_insert_default().extend(value);
} else if self.attr_class.contains(&attr) {
for value in value {
is_group |= value.eq_ignore_ascii_case(&self.group_class);

View File

@@ -234,19 +234,18 @@ impl OpenIdDirectory {
email,
email_aliases: Vec::new(),
secret: None,
groups: self
.config
.claim_groups
.as_ref()
.and_then(|groups_claim| claims.get(groups_claim))
.map(extract_string_list)
.unwrap_or_default()
.into_iter()
.map(|group| match &self.config.default_domain {
Some(domain) if !group.contains('@') => format!("{group}@{domain}"),
_ => group,
})
.collect(),
groups: self.config.claim_groups.as_ref().map(|groups_claim| {
claims
.get(groups_claim)
.map(extract_string_list)
.unwrap_or_default()
.into_iter()
.map(|group| match &self.config.default_domain {
Some(domain) if !group.contains('@') => format!("{group}@{domain}"),
_ => group,
})
.collect()
}),
description: self
.config
.claim_name

View File

@@ -52,6 +52,7 @@ impl SqlDirectory {
// Obtain members
if let Some(query) = &self.mappings.query_member_of {
let members = account.groups.get_or_insert_default();
for row in self
.sql_store
.sql_query::<Rows>(query, vec![username.into()])
@@ -62,7 +63,7 @@ impl SqlDirectory {
if let Some(Value::Text(address)) = row.values.first()
&& let Some(email) = sanitize_email(address)
{
account.groups.push(email);
members.push(email);
}
}
}
@@ -103,6 +104,7 @@ impl SqlDirectory {
Recipient::Account(mut account) => {
// Obtain members
if let Some(query) = &self.mappings.query_member_of {
let members = account.groups.get_or_insert_default();
for row in self
.sql_store
.sql_query::<Rows>(query, vec![account.email.as_str().into()])
@@ -113,7 +115,7 @@ impl SqlDirectory {
if let Some(Value::Text(address)) = row.values.first()
&& let Some(email) = sanitize_email(address)
{
account.groups.push(email);
members.push(email);
}
}
}

View File

@@ -47,7 +47,7 @@ pub struct Account {
pub email: String,
pub email_aliases: Vec<String>,
pub secret: Option<String>,
pub groups: Vec<String>,
pub groups: Option<Vec<String>>,
pub description: Option<String>,
}

View File

@@ -29,7 +29,7 @@ pub async fn test() {
email: "john.doe@example.org".into(),
email_aliases: vec!["john@example.org".into()],
secret: Some("$app$8958830913002348890$".into()),
groups: vec!["sales@example.org".into()],
groups: Some(vec!["sales@example.org".into()]),
description: Some("John Doe".into()),
}
);
@@ -45,7 +45,7 @@ pub async fn test() {
email: "jane.smith@example.org".into(),
email_aliases: vec![],
secret: Some("$app$4096614298472586996$".into()),
groups: vec!["sales@example.org".into(), "corporate@example.org".into()],
groups: Some(vec!["sales@example.org".into(), "corporate@example.org".into()]),
description: Some("Jane Smith".into()),
}
);
@@ -76,7 +76,7 @@ pub async fn test() {
email: "john.doe@example.org".into(),
email_aliases: vec!["john@example.org".into()],
secret: Some("this is John's LDAP password".into()),
groups: vec!["sales@example.org".into()],
groups: Some(vec!["sales@example.org".into()]),
description: Some("John Doe".into()),
}
);
@@ -97,7 +97,7 @@ pub async fn test() {
email: "john.doe@example.org".into(),
email_aliases: vec!["john@example.org".into()],
secret: Some("this is John's LDAP password".into()),
groups: vec!["sales@example.org".into()],
groups: Some(vec!["sales@example.org".into()]),
description: Some("John Doe".into())
})
);
@@ -107,7 +107,7 @@ pub async fn test() {
email: "jane.smith@example.org".into(),
email_aliases: vec![],
secret: Some("this is Jane's LDAP password".into()),
groups: vec!["sales@example.org".into(), "corporate@example.org".into()],
groups: Some(vec!["sales@example.org".into(), "corporate@example.org".into()]),
description: Some("Jane Smith".into())
})
);

View File

@@ -44,7 +44,7 @@ pub async fn test() {
email: "john.doe@example.org".to_string(),
email_aliases: vec![],
secret: None,
groups: vec!["sales@example.org".to_string()],
groups: Some(vec!["sales@example.org".to_string()]),
description: Some("John Doe".to_string())
}
);
@@ -66,7 +66,7 @@ pub async fn test() {
email: "john.doe@example.org".to_string(),
email_aliases: vec![],
secret: None,
groups: vec!["sales@example.org".to_string()],
groups: Some(vec!["sales@example.org".to_string()]),
description: Some("John Doe".to_string())
}
);
@@ -115,7 +115,7 @@ pub async fn test() {
email: "john.doe@example.org".to_string(),
email_aliases: vec![],
secret: None,
groups: vec!["sales@example.org".to_string()],
groups: Some(vec!["sales@example.org".to_string()]),
description: None,
}
);

View File

@@ -97,7 +97,7 @@ pub async fn test() {
email: "john@example.org".to_string(),
email_aliases: vec!["john.doe@example.org".to_string(),],
secret: Some("john secret".to_string()),
groups: vec!["sales@example.org".to_string()],
groups: Some(vec!["sales@example.org".to_string()]),
description: Some("John Doe".to_string()),
}
);
@@ -118,7 +118,7 @@ pub async fn test() {
email: "john@example.org".to_string(),
email_aliases: vec!["john.doe@example.org".to_string()],
secret: Some("john secret".to_string()),
groups: vec!["sales@example.org".to_string()],
groups: Some(vec!["sales@example.org".to_string()]),
description: Some("John Doe".to_string()),
})
);
@@ -128,7 +128,7 @@ pub async fn test() {
email: "jane@example.org".to_string(),
email_aliases: vec![],
secret: Some("jane secret".to_string()),
groups: vec!["sales@example.org".to_string()],
groups: Some(vec!["sales@example.org".to_string()]),
description: Some("Jane Doe".to_string()),
})
);

View File

@@ -29,7 +29,7 @@ pub async fn test() {
email: "john@unknown.org".to_string(),
email_aliases: vec![],
secret: "supersecret".to_string().into(),
groups: vec![],
groups: Some(vec![]),
description: "John Doe".to_string().into(),
})
.await
@@ -44,10 +44,10 @@ pub async fn test() {
"j.doe@example.org".to_string(),
],
secret: "supersecret".to_string().into(),
groups: vec![
groups: Some(vec![
"corporate@example.org".to_string(),
"sales@example.org".to_string(),
],
]),
description: "John Doe".to_string().into(),
};
let result = test
@@ -162,8 +162,9 @@ pub async fn test() {
account_in
.email_aliases
.push("johnny@example.org".to_string());
account_in.groups.pop();
account_in.groups.push("support@example.org".to_string());
let groups = account_in.groups.get_or_insert_default();
groups.pop();
groups.push("support@example.org".to_string());
account_in.secret = "evenmoresecret".to_string().into();
assert_eq!(
test.server
@@ -227,6 +228,38 @@ pub async fn test() {
4
);
account_in.groups = None;
test.server
.synchronize_account(account_in.clone())
.await
.unwrap();
let account_out = test
.server
.registry()
.object::<Account>(account_id)
.await
.unwrap()
.unwrap()
.into_user()
.unwrap();
assert_eq!(account_out.member_group_ids.len(), 2);
account_in.groups = Some(vec![]);
test.server
.synchronize_account(account_in.clone())
.await
.unwrap();
let account_out = test
.server
.registry()
.object::<Account>(account_id)
.await
.unwrap()
.unwrap()
.into_user()
.unwrap();
assert_eq!(account_out.member_group_ids.len(), 0);
// Synchronize a group
assert_eq!(
test.server