diff --git a/crates/common/src/hold.rs b/crates/common/src/hold.rs index c97dd56..cfa7325 100644 --- a/crates/common/src/hold.rs +++ b/crates/common/src/hold.rs @@ -15,7 +15,14 @@ use inbuxa_features::{ hold::{self, HELD_UNTIL, Hold, Keeping, Member, is_held_until}, undelete::records, }; -use registry::schema::{prelude::ObjectType, structs::ArchivedItem}; +use inbuxa_features::undelete::data::{self as undelete_data, KeptAccount}; +use registry::{ + pickle::PickledStream, + schema::{ + prelude::{ObjectInner, ObjectType}, + structs::ArchivedItem, + }, +}; use store::{registry::RegistryQuery, write::now}; use trc::AddContext; use types::id::Id; @@ -29,6 +36,21 @@ const RELEASE_GRACE: u64 = 30 * 86_400; pub struct Settled { pub frozen: usize, pub released: usize, + /// Deleted accounts kept by a hold, or let go by a release (LH-8, LH-10). + pub accounts_frozen: usize, + pub accounts_released: usize, +} + +/// A kept account as it was when deleted, for a hold's scope: its record +/// still names its domain, groups and tenant. +pub fn kept_member(account_id: u32, kept: &KeptAccount) -> Member { + PickledStream::new(&kept.record) + .and_then(|mut stream| ObjectInner::unpickle(ObjectType::Account, &mut stream)) + .and_then(|inner| Member::of(account_id, &inner)) + .unwrap_or(Member { + account: account_id, + ..Default::default() + }) } impl Server { @@ -114,9 +136,41 @@ impl Server { settled.released += 1; } } + + // LH-8, LH-10: deleted accounts kept by undelete follow the holds + // too. Their DestroyAccount task defers itself while they're kept. + let retention = inbuxa_features::undelete::settings::retention(registry) + .await? + .accounts; + for (account_id, mut kept) in undelete_data::kept_accounts(data).await? { + let covered = !hold::covering(data, &kept_member(account_id, &kept)).await?.is_empty(); + let held = is_held_until(kept.kept_until); + let until = if covered && !held { + settled.accounts_frozen += 1; + HELD_UNTIL + } else if !covered && held { + settled.accounts_released += 1; + (kept.deleted_at + retention.unwrap_or(0)).max(now + RELEASE_GRACE) + } else { + continue; + }; + kept.kept_until = until; + let mut batch = store::write::BatchBuilder::new(); + undelete_data::set_kept_account(&mut batch, account_id, &kept)?; + data.write(batch.build_all()) + .await + .caused_by(trc::location!())?; + } Ok(settled) } + /// LH-8: whether a hold covers a deleted account undelete keeps. + pub async fn is_kept_held(&self, account_id: u32, kept: &KeptAccount) -> trc::Result { + Ok(!hold::covering(self.store(), &kept_member(account_id, kept)) + .await? + .is_empty()) + } + /// Every account an active hold covers now. Empty, without looking at /// accounts, when nothing is held. pub async fn held_accounts(&self) -> trc::Result> { diff --git a/crates/features/src/hold/mod.rs b/crates/features/src/hold/mod.rs index 5815647..cdee60f 100644 --- a/crates/features/src/hold/mod.rs +++ b/crates/features/src/hold/mod.rs @@ -484,6 +484,19 @@ pub async fn keep_moved(data: &Store, before: &Member, after: &Member) -> trc::R Ok(()) } +/// LH-8: names `account_id` in every hold that reaches it, so a deleted +/// account, no longer in any domain or tenant, stays held. +pub async fn pin_account(data: &Store, member: &Member) -> trc::Result<()> { + for mut hold in covering(data, member).await? { + if !hold.scope.accounts.contains(&member.account) { + hold.scope.accounts.push(member.account); + hold.scope.accounts.sort_unstable(); + 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(); diff --git a/crates/jmap/src/inbuxa/deleted_account.rs b/crates/jmap/src/inbuxa/deleted_account.rs index a635bae..bbfb07c 100644 --- a/crates/jmap/src/inbuxa/deleted_account.rs +++ b/crates/jmap/src/inbuxa/deleted_account.rs @@ -124,13 +124,29 @@ pub async fn reserved( /// of upstream's immediate destruction. Returns the other accounts whose /// access changed, or `None` when nothing is kept. pub async fn keep(server: &Server, id: Id, account: &Account) -> trc::Result>> { - let Some(period) = retention(server.registry()).await?.accounts else { - return Ok(None); - }; let account_id = id.document_id(); - let deleted_at = now(); - let kept_until = deleted_at + period; let inner = ObjectInner::Account(account.clone()); + // inbuxa: LH-8: a held account's data stays, with no expiry, whether or + // not undelete keeps accounts; its holds name it from now on + let member = inbuxa_features::hold::Member::of(account_id, &inner); + let held = match &member { + Some(member) => { + !inbuxa_features::hold::covering(server.store(), member) + .await? + .is_empty() + } + None => false, + }; + let period = retention(server.registry()).await?.accounts; + let deleted_at = now(); + let kept_until = match (held, period) { + (true, _) => inbuxa_features::hold::HELD_UNTIL, + (false, Some(period)) => deleted_at + period, + (false, None) => return Ok(None), + }; + if held && let Some(member) = &member { + inbuxa_features::hold::pin_account(server.store(), member).await?; + } let addresses = addresses_of(server, &inner) .await? .into_iter() @@ -324,6 +340,18 @@ pub async fn set( for id in will_destroy { match data::kept_account(data, id.document_id()).await? { + // inbuxa: LH-8: a held account's data can't be destroyed + Some(kept) + if may_reach(access_token, &kept, Permission::SysAccountDestroy) + && (inbuxa_features::hold::is_held_until(kept.kept_until) + || server.is_kept_held(id.document_id(), &kept).await?) => + { + response.not_destroyed.append( + id, + SetError::forbidden() + .with_description("A legal hold applies to this account, so its data stays."), + ); + } Some(kept) if may_reach(access_token, &kept, Permission::SysAccountDestroy) => { destroy_now(server, id, &kept).await?; response.destroyed.push(id); diff --git a/crates/services/src/task_manager/destroy_account.rs b/crates/services/src/task_manager/destroy_account.rs index ee88695..1d3da5a 100644 --- a/crates/services/src/task_manager/destroy_account.rs +++ b/crates/services/src/task_manager/destroy_account.rs @@ -55,6 +55,23 @@ impl DestroyAccountTask for Server { async fn destroy_account(server: &Server, task: &TaskDestroyAccount) -> trc::Result { let account_id = task.account_id.document_id(); + // inbuxa: LH-8, LH-10: a kept account waits for its time, and a held one + // for its release; "destroy now" clears the kept record first + if let Some(kept) = + inbuxa_features::undelete::data::kept_account(&server.core.storage.data, account_id).await? + { + let now = store::write::now(); + let held = inbuxa_features::hold::is_held_until(kept.kept_until) + || server.is_kept_held(account_id, &kept).await?; + if held || kept.kept_until > now { + let retry = if held { now + 86_400 } else { kept.kept_until }; + return Ok(TaskResult::deferred( + Some(retry), + "The account is still kept: a legal hold applies, or its time hasn't come.", + )); + } + } + // Destroy public keys and masked emails for object in [ObjectType::PublicKey, ObjectType::MaskedEmail] { let mut batch = BatchBuilder::new(); diff --git a/tests/src/system/legal_hold.rs b/tests/src/system/legal_hold.rs index 7e265c8..219f115 100644 --- a/tests/src/system/legal_hold.rs +++ b/tests/src/system/legal_hold.rs @@ -309,7 +309,10 @@ pub async fn test(test: &mut TestServer) { "r": {"name": "Matter 6002", "from": "2020-01-01T00:00:00Z", "to": "2020-12-31T23:59:59Z", "scope": {"accounts": [ranged.id_string()]}}}})) .await; - assert!(response["created"]["w"]["id"].is_string(), "LH-1: {response}"); + let whole_hold = response["created"]["w"]["id"] + .as_str() + .unwrap_or_else(|| panic!("LH-1: {response}")) + .to_string(); assert!(response["created"]["r"]["id"].is_string(), "LH-1: {response}"); let held_client = held.jmap_client().await; @@ -450,6 +453,52 @@ pub async fn test(test: &mut TestServer) { "test 8, LH-10: under 30 days of grace after release: {until}" ); + // Test 8, LH-8: a held account destroyed as a login is kept, data and + // all, with no expiry, although undelete keeps no accounts here + let held_id = held.id_string().to_string(); + admin.destroy_account(held).await; + let kept = |list: Value| { + list["list"] + .as_array() + .and_then(|l| l.iter().find(|a| a["id"] == held_id.as_str()).cloned()) + }; + let (_, list) = admin + .hold_call("inbuxa:DeletedAccount/get", json!({"ids": null})) + .await; + let entry = kept(list.clone()).unwrap_or_else(|| panic!("test 8, LH-8: not kept: {list}")); + assert!( + entry["keptUntil"].as_str().is_some_and(|u| u.starts_with("9999-")), + "test 8, LH-8: kept with an expiry: {entry}" + ); + let (_, response) = admin + .hold_call("inbuxa:DeletedAccount/set", json!({"destroy": [held_id]})) + .await; + assert_eq!( + response["notDestroyed"][held_id.as_str()]["type"], "forbidden", + "test 8, LH-8: destroy-now wasn't refused: {response}" + ); + // Its hold names it now, so no domain or tenant move can drop it + assert!( + admin.hold_get(&whole_hold).await["scope"]["accounts"] + .as_array() + .is_some_and(|a| a.iter().any(|id| id == held_id.as_str())), + "LH-8: the hold doesn't name the deleted account" + ); + // Release: the data is destroyed 30 days later, not before + admin + .hold_set(json!({"reason": "Matter closed", "update": {whole_hold.as_str(): {"released": true}}})) + .await; + let (_, list) = admin + .hold_call("inbuxa:DeletedAccount/get", json!({"ids": null})) + .await; + let entry = kept(list.clone()).unwrap_or_else(|| panic!("test 8, LH-10: gone at release: {list}")); + let until = entry["keptUntil"].as_str().unwrap_or_default().to_string(); + let grace = chrono::Utc::now() + chrono::Duration::days(29); + assert!( + !until.starts_with("9999-") && until > grace.format("%Y-%m-%dT%H:%M:%S").to_string(), + "test 8, LH-10: after release, not 30 days of grace: {until}" + ); + // 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}}}))