From c1b5bf956cb01ba71bd52a19a3115888f280fd7c Mon Sep 17 00:00:00 2001 From: John Coffey Date: Sun, 27 Sep 2026 19:28:39 -0700 Subject: [PATCH] Audit records name accounts in full, and holds by name An account's or mailing list's name is only its local part, so the log said "Account ken.gosling" where two domains could each have one; it now says ken.gosling@alpineski.test. A change to a legal hold was recorded under its id; the hold's current state is now read first, so the record carries its case name and each change reads before/after. --- crates/jmap/src/inbuxa/audit.rs | 64 +++++++++++++++++++++++++++++++-- tests/src/system/legal_hold.rs | 27 ++++++++++++++ 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/crates/jmap/src/inbuxa/audit.rs b/crates/jmap/src/inbuxa/audit.rs index 2fc61fa..7e397a3 100644 --- a/crates/jmap/src/inbuxa/audit.rs +++ b/crates/jmap/src/inbuxa/audit.rs @@ -167,7 +167,8 @@ async fn before( for (client_id, value) in request.create.iter().flat_map(|c| c.iter()) { let after = serde_json::to_value(value).unwrap_or_default(); - let described = diff::describe(&after); + let mut described = diff::describe(&after); + described.name = full_name(server, object, &after, described.name).await; let changes = after .as_object() .map(|patch| diff::patch(object, None, patch)) @@ -192,7 +193,10 @@ async fn before( None => fork_current(server, object, id).await, }; let patch = serde_json::to_value(value).unwrap_or_default(); - let described = before.as_ref().map(diff::describe).unwrap_or_default(); + let mut described = before.as_ref().map(diff::describe).unwrap_or_default(); + if let Some(before) = &before { + described.name = full_name(server, object, before, described.name).await; + } let changes = patch .as_object() .map(|patch| diff::patch(object, before.as_ref(), patch)) @@ -214,7 +218,10 @@ async fn before( if let Some(MaybeResultReference::Value(destroy)) = &request.destroy { for id in destroy { let before = stored(server, registry, id).await; - let described = before.as_ref().map(diff::describe).unwrap_or_default(); + let mut described = before.as_ref().map(diff::describe).unwrap_or_default(); + if let Some(before) = &before { + described.name = full_name(server, object, before, described.name).await; + } records.push(( Item::Destroy(id.clone()), Action::Destroy, @@ -345,6 +352,29 @@ fn id_text(id: &MaybeInvalid) -> String { /// The fork's own settings as they are now, as JSON, so their changes are /// recorded with what they replaced. Their stored names are the JMAP /// property names. +/// An account's or a mailing list's name is only its local part, and two +/// domains' "leslie" would read alike: records name it by its full address. +async fn full_name(server: &Server, object: &str, value: &Value, name: Option) -> Option { + let name = name?; + if !matches!(object, "x:Account" | "x:MailingList") || name.contains('@') { + return Some(name); + } + let domain = value + .get("domainId") + .and_then(Value::as_str) + .and_then(|id| ::from_str(id).ok()); + match domain { + Some(domain) => match server.domain_by_id(domain.document_id()).await { + Ok(Some(domain)) => match domain.names.first() { + Some(domain) => Some(format!("{name}@{domain}")), + None => Some(name), + }, + _ => Some(name), + }, + None => Some(name), + } +} + async fn fork_current(server: &Server, object: &str, id: &MaybeInvalid) -> Option { use inbuxa_features::{ai::limits, audit::log, security}; let data = server.store(); @@ -361,6 +391,34 @@ async fn fork_current(server: &Server, object: &str, id: &MaybeInvalid) -> O .await .ok() .and_then(|policy| serde_json::to_value(policy).ok()), + // LH-1: a hold as the API shows it, so a change reads before/after + "inbuxa:LegalHold" => match id { + MaybeInvalid::Value(id) => { + let hold = inbuxa_features::hold::get(data, u32::try_from(id.id()).ok()?) + .await + .ok()??; + let ids = |list: &[u32]| list.iter().map(|id| Id::from(*id).to_string()).collect::>(); + let date = |at: Option| { + at.map(|at| jmap_proto::types::date::UTCDate::from_timestamp(at as i64).to_string()) + }; + Some(serde_json::json!({ + "name": hold.name, + "reference": hold.reference, + "description": hold.description, + "scope": { + "server": hold.scope.server, + "accounts": ids(&hold.scope.accounts), + "groups": ids(&hold.scope.groups), + "domains": ids(&hold.scope.domains), + "tenants": ids(&hold.scope.tenants), + }, + "from": date(hold.from), + "to": date(hold.to), + "released": !hold.is_active(), + })) + } + MaybeInvalid::Invalid(_) => None, + }, "inbuxa:TenantProtocolPolicy" => match id { MaybeInvalid::Value(id) => { security::tenant_protocol_policy::get(data, id.document_id()) diff --git a/tests/src/system/legal_hold.rs b/tests/src/system/legal_hold.rs index e7716df..cd1d523 100644 --- a/tests/src/system/legal_hold.rs +++ b/tests/src/system/legal_hold.rs @@ -574,6 +574,33 @@ pub async fn test(test: &mut TestServer) { for reason in ["Counsel's letter", "Counsel widened the matter", "Matter settled"] { assert!(reasons.contains(&reason), "AU-12: {reason:?} not recorded: {reasons:?}"); } + // A release is recorded under the hold's name, from before to after + let list = records["list"].as_array().cloned().unwrap_or_default(); + let release = list + .iter() + .find(|r| r["reason"] == "First settled") + .unwrap_or_else(|| panic!("the first release isn't recorded: {list:?}")); + assert_eq!(release["target"]["name"], "Matter 7001", "{release}"); + assert!( + release["changes"] + .as_array() + .is_some_and(|c| c.iter().any(|c| c["field"] == "released" && c["before"] == false && c["after"] == true)), + "the release doesn't read before/after: {release}" + ); + + // Accounts are named by their full address, not the bare local part + let (_, query) = admin + .hold_call("inbuxa:AuditEvent/query", json!({"filter": {"targetKind": "x:Account"}})) + .await; + let (_, accounts) = admin + .hold_call("inbuxa:AuditEvent/get", json!({"ids": query["ids"].clone()})) + .await; + assert!( + accounts["list"] + .as_array() + .is_some_and(|l| l.iter().any(|r| r["target"]["name"] == "held@example.com")), + "an account isn't named by its address: {accounts}" + ); } async fn import(