From 318783f4446d36ca7a5b291aad1fbe5e5f8758a1 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Sun, 27 Sep 2026 17:56:02 -0700 Subject: [PATCH] Legal holds, step 2: who a hold covers A hold reaches an account by name, through any of its addresses' domains, its groups or its tenant, as they are now, so an account added to a held domain later is held too. An account that leaves a held domain, group or tenant stays held: the registry write hook adds it to the hold by name on every account change, whoever makes it (LH-2). Server::holds_on answers for the deletion paths, from the store each time so a hold binds every node at once. --- crates/common/src/audit.rs | 15 ++++++ crates/common/src/hold.rs | 43 +++++++++++++++ crates/common/src/lib.rs | 1 + crates/features/src/hold/mod.rs | 92 +++++++++++++++++++++++++++++++++ tests/src/system/legal_hold.rs | 51 ++++++++++++++++++ 5 files changed, 202 insertions(+) create mode 100644 crates/common/src/hold.rs diff --git a/crates/common/src/audit.rs b/crates/common/src/audit.rs index 5eac3aa..1cfeb90 100644 --- a/crates/common/src/audit.rs +++ b/crates/common/src/audit.rs @@ -14,6 +14,7 @@ use crate::{ auth::{AccessToken, AuthRequest, permissions::DefaultPermissions}, }; use directory::Credentials; +use inbuxa_features::hold::{self, Member}; use inbuxa_features::audit::{ Action, Actor, AuditLog, EntryId, Outcome, Record, Target, Via, diff, log, scope, }; @@ -495,6 +496,20 @@ impl RegistryWriteHook for SystemWrites { change: RegistryChange<'a>, ) -> Pin + Send + 'a>> { Box::pin(async move { + // LH-2: every change to an account, whoever makes it: one that + // leaves a held domain, group or tenant stays held by name + if change.object_type == ObjectType::Account + && let (Some(before), Some(after)) = (change.before, change.after) + && let (Some(before), Some(after)) = ( + Member::of(change.id.document_id(), &before.inner), + Member::of(change.id.document_id(), &after.inner), + ) + && let Err(err) = hold::keep_moved(&self.data, &before, &after).await + { + trc::error!(err + .account_id(after.account) + .details("Failed to keep a moved account under its legal hold")); + } let subsystem = match scope::current() { Some(scope::Scope::Request | scope::Scope::Quiet) => return, Some(scope::Scope::System(subsystem)) => subsystem, diff --git a/crates/common/src/hold.rs b/crates/common/src/hold.rs new file mode 100644 index 0000000..58be497 --- /dev/null +++ b/crates/common/src/hold.rs @@ -0,0 +1,43 @@ +/* + * SPDX-FileCopyrightText: 2026 Coffey Labs + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +//! inbuxa: which legal holds cover an account (audit-hold-lock spec, LH-2, +//! LH-11), for the paths that destroy data. Read from the store every time, +//! not cached: a hold placed on one node must bind every node at once, and +//! there are few holds. + +use crate::Server; +use inbuxa_features::hold::{self, Hold, Member}; + +impl Server { + /// The active holds covering `account_id`, through its own name, its + /// addresses' domains, its groups or its tenant. Empty for an account + /// that no longer exists: a deleted one is kept by LH-8's own check. + pub async fn holds_on(&self, account_id: u32) -> trc::Result> { + let Ok(account) = self.account(account_id).await else { + return Ok(Vec::new()); + }; + let mut domains = account + .addresses + .iter() + .map(|address| address.domain_id) + .collect::>(); + domains.sort_unstable(); + domains.dedup(); + let member = Member { + account: account_id, + domains, + groups: account.id_member_of.iter().copied().collect(), + tenant: account.id_tenant, + }; + hold::covering(self.store(), &member).await + } + + /// Whether any active hold covers `account_id` at all. + pub async fn is_held(&self, account_id: u32) -> trc::Result { + Ok(!self.holds_on(account_id).await?.is_empty()) + } +} diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs index 54b0f53..bbf2d71 100644 --- a/crates/common/src/lib.rs +++ b/crates/common/src/lib.rs @@ -68,6 +68,7 @@ use utils::{ pub mod auth; pub mod cache; pub mod audit; // inbuxa: the audit log (audit-hold-lock spec, AU) +pub mod hold; // inbuxa: legal holds (audit-hold-lock spec, LH) pub mod config; pub mod expr; pub mod i18n; diff --git a/crates/features/src/hold/mod.rs b/crates/features/src/hold/mod.rs index 1792e82..b940b47 100644 --- a/crates/features/src/hold/mod.rs +++ b/crates/features/src/hold/mod.rs @@ -18,6 +18,7 @@ //! //! Numbers are big-endian. There are few holds, so they're read whole. +use registry::schema::{prelude::ObjectInner, structs::Account}; use serde::{Deserialize as SerdeDeserialize, Serialize as SerdeSerialize}; use store::{ Deserialize, IterateParams, SUBSPACE_INBUXA, Serialize, Store, ValueKey, @@ -83,6 +84,48 @@ impl Scope { } } +/// What decides whether a hold's scope reaches an account: the domains of +/// its addresses, its groups and its tenant (LH-2). +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct Member { + pub account: u32, + pub domains: Vec, + pub groups: Vec, + pub tenant: Option, +} + +impl Member { + /// A person's account as the registry stores it; `None` for a group, + /// whose own data is held through its members. + pub fn of(account_id: u32, object: &ObjectInner) -> Option { + let ObjectInner::Account(Account::User(user)) = object else { + return None; + }; + let mut domains = vec![user.domain_id.document_id()]; + domains.extend(user.aliases.iter().map(|alias| alias.domain_id.document_id())); + domains.sort_unstable(); + domains.dedup(); + Some(Member { + account: account_id, + domains, + groups: user.member_group_ids.iter().map(|id| id.document_id()).collect(), + tenant: user.member_tenant_id.map(|id| id.document_id()), + }) + } +} + +impl Scope { + /// Whether this scope reaches `member`, directly or through its domains, + /// groups or tenant, as they are now (LH-2). + pub fn covers(&self, member: &Member) -> bool { + self.server + || self.accounts.contains(&member.account) + || member.domains.iter().any(|d| self.domains.contains(d)) + || member.groups.iter().any(|g| self.groups.contains(g)) + || member.tenant.is_some_and(|t| self.tenants.contains(&t)) + } +} + /// When and why a hold was released (LH-10). #[derive(Debug, Clone, PartialEq, Eq, SerdeSerialize, SerdeDeserialize)] #[serde(rename_all = "camelCase")] @@ -300,6 +343,33 @@ pub async fn create(data: &Store, hold: &Hold) -> trc::Result { } } +/// The active holds that reach `member` (LH-2, LH-11). +pub async fn covering(data: &Store, member: &Member) -> trc::Result> { + Ok(active(data) + .await? + .into_iter() + .filter(|hold| hold.scope.covers(member)) + .collect()) +} + +/// LH-2: an account a hold reached through its domain, group or tenant stays +/// held when it leaves them: it is added to the hold by name. Called for +/// every change to an account, so no move escapes a hold. +pub async fn keep_moved(data: &Store, before: &Member, after: &Member) -> trc::Result<()> { + if before == after { + return Ok(()); + } + for mut hold in active(data).await? { + if hold.scope.covers(before) && !hold.scope.covers(after) { + hold.scope.accounts.push(after.account); + hold.scope.accounts.sort_unstable(); + hold.scope.accounts.dedup(); + update(data, &hold).await?; + } + } + Ok(()) +} + /// Replaces a hold that `check_update` allowed. pub async fn update(data: &Store, hold: &Hold) -> trc::Result<()> { let mut batch = BatchBuilder::new(); @@ -419,6 +489,28 @@ mod tests { assert!(open_ended.covers_date(u64::MAX), "no `to` also catches mail still to come"); } + #[test] + fn a_scope_reaches_members_through_domain_group_and_tenant() { + let member = Member { + account: 9, + domains: vec![3, 4], + groups: vec![20], + tenant: Some(7), + }; + let reaches = |scope: Scope| scope.covers(&member); + assert!(reaches(accounts(&[9]))); + assert!(reaches(Scope { domains: vec![4], ..Default::default() }), "an alias's domain counts"); + assert!(reaches(Scope { groups: vec![20], ..Default::default() })); + assert!(reaches(Scope { tenants: vec![7], ..Default::default() })); + assert!(reaches(Scope { server: true, ..Default::default() })); + assert!(!reaches(Scope { domains: vec![5], tenants: vec![8], ..Default::default() })); + + // LH-2: leaving the held domain would free it, so the hold must name it + let held = hold(Scope { domains: vec![3], ..Default::default() }, None, None); + let moved = Member { domains: vec![6], ..member.clone() }; + assert!(held.scope.covers(&member) && !held.scope.covers(&moved)); + } + #[test] fn stored_as_json() { let current = hold(accounts(&[2]), Some(100), None); diff --git a/tests/src/system/legal_hold.rs b/tests/src/system/legal_hold.rs index 76f3826..f6f524e 100644 --- a/tests/src/system/legal_hold.rs +++ b/tests/src/system/legal_hold.rs @@ -215,6 +215,57 @@ pub async fn test(test: &mut TestServer) { .await; assert_eq!(name, "error", "LH-13: a tenant administrator placed a hold: {response}"); + // Test 7, LH-2: a hold on a domain reaches an account created there + // later, and keeps it by name when it moves to another domain + let held_domain = admin + .registry_create_object(Domain { + name: "held.example.net".to_string(), + is_enabled: true, + certificate_management: CertificateManagement::Manual, + dns_management: DnsManagement::Manual, + dkim_management: DkimManagement::Manual, + ..Default::default() + }) + .await; + let elsewhere = admin + .registry_create_object(Domain { + name: "elsewhere.example.net".to_string(), + is_enabled: true, + certificate_management: CertificateManagement::Manual, + dns_management: DnsManagement::Manual, + dkim_management: DkimManagement::Manual, + ..Default::default() + }) + .await; + let response = admin + .hold_set(json!({"reason": "Whole division", "create": {"d": { + "name": "Matter 5120", "scope": {"domains": [held_domain.to_string()]}}}})) + .await; + let domain_hold = response["created"]["d"]["id"] + .as_str() + .unwrap_or_else(|| panic!("LH-1 domain hold: {response}")) + .to_string(); + let mover = admin + .create_user_account("mover@held.example.net", "mover-secret-8812", "Mover", &[], vec![]) + .await; + assert_eq!( + admin.hold_get(&domain_hold).await["scope"]["accounts"], + json!([]), + "LH-2: covered through the domain, not named yet" + ); + admin + .registry_update_object( + ObjectType::Account, + mover.id(), + json!({Property::DomainId: elsewhere.to_string()}), + ) + .await; + assert_eq!( + admin.hold_get(&domain_hold).await["scope"]["accounts"], + json!([mover.id_string()]), + "test 7, LH-2: the moved account escaped the hold" + ); + // LH-10: release needs a reason, and a released hold stays, read-only let response = admin .hold_set(json!({"update": {hold_id.as_str(): {"released": true}}}))