From 4a9aa9c54813ee8bfb90f7a6b1d44c426e07af21 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Fri, 18 Sep 2026 18:29:19 -0700 Subject: [PATCH] Masked email: rewrite to the owner at RCPT TO, create responses carry the address, admins query all masks (ME-4, ME-9, ME-13, ME-19) Found by running system_tests, which masked email no longer stops: - rcpt_resolve rewrites a live mask to its owner's address, so Delivered-To names the account; delivery recognizes the mask from the original recipient when it belongs to that account. - x:MaskedEmail/set create responses carry the server-set email. - x:MaskedEmail/query returns every mask to a server-level impersonate holder, and filters on accountId. - The refusal for an unlinked emailDomain uses upstream's wording. - The shared delivery test checks the fork's address format (ME-13). - The masked email test's tenant domain uses manual DKIM, so its cleanup leaves nothing behind. --- crates/common/src/network/mta.rs | 21 +++++++ crates/email/src/message/delivery.rs | 18 +++++- crates/features/src/masked_email/ops.rs | 39 ++++++++++++ crates/jmap/src/inbuxa/masked_email.rs | 81 ++++++++++++++++--------- docs/spec/features/masked-email.md | 12 +++- tests/src/system/delivery.rs | 5 +- tests/src/system/masked_email.rs | 19 +++++- 7 files changed, 160 insertions(+), 35 deletions(-) diff --git a/crates/common/src/network/mta.rs b/crates/common/src/network/mta.rs index c0ddd07..249c36b 100644 --- a/crates/common/src/network/mta.rs +++ b/crates/common/src/network/mta.rs @@ -72,6 +72,27 @@ impl Server { } } + // inbuxa: ME-4, ME-9: a live masked address is rewritten to its + // owner's, which keeps the mask as the original recipient + if let inbuxa_features::masked_email::ops::Lookup::Accepts(mask) = + inbuxa_features::masked_email::ops::lookup( + &self.core.storage.data, + self.registry(), + &format!("{local_part}@{domain_part}"), + ) + .await? + { + let owner = self.account(mask.object.account_id.document_id()).await?; + if let Some(address) = owner.addresses.first() + && let Some(owner_domain) = self.domain_by_id(address.domain_id).await? + && let Some(owner_domain) = owner_domain.names.first() + { + return Ok(RcptResolution::Rewrite(format!( + "{}@{}", + address.local_part, owner_domain + ))); + } + } // Obtain external directory, if configured let directory = self diff --git a/crates/email/src/message/delivery.rs b/crates/email/src/message/delivery.rs index f552919..987d988 100644 --- a/crates/email/src/message/delivery.rs +++ b/crates/email/src/message/delivery.rs @@ -130,7 +130,7 @@ impl MailDelivery for Server { for rcpt in message.recipients { // inbuxa: ME-4, ME-10: a masked address delivers to its owner - let mask = match inbuxa_features::masked_email::ops::resolve_recipient( + let mut mask = match inbuxa_features::masked_email::ops::resolve_recipient( &self.core.storage.data, self.registry(), &rcpt.address, @@ -177,6 +177,22 @@ impl MailDelivery for Server { continue; } }; + // inbuxa: ME-9: rewritten at RCPT TO, the mask is the original recipient + if mask.is_none() { + match inbuxa_features::masked_email::ops::resolve_original( + &self.core.storage.data, + self.registry(), + rcpt.orcpt.as_deref(), + account_id, + ) + .await + { + Ok(original) => mask = original, + Err(err) => { + trc::error!(err.span_id(message.session_id)); + } + } + } if let Some(status) = account_ids .get(&account_id) .and_then(|pos| result.status.get(*pos)) diff --git a/crates/features/src/masked_email/ops.rs b/crates/features/src/masked_email/ops.rs index 34e6f72..4624ed7 100644 --- a/crates/features/src/masked_email/ops.rs +++ b/crates/features/src/masked_email/ops.rs @@ -105,6 +105,21 @@ pub async fn of_account( Ok(masks) } +/// Every mask on the server, for a server-level administrator (ME-19). +pub async fn all(data: &Store, registry: &RegistryStore) -> trc::Result> { + let mut masks = Vec::new(); + for id in registry + .query::>(RegistryQuery::new(ObjectType::MaskedEmail)) + .await + .caused_by(trc::location!())? + { + if let Some(mask) = load(data, registry, id).await? { + masks.push(mask); + } + } + Ok(masks) +} + /// How many masks count against the account's limit (ME-14). pub async fn live_count( data: &Store, @@ -440,6 +455,30 @@ pub async fn resolve_recipient( Ok(None) } +/// The mask a delivery came through, when the recipient was rewritten from +/// a mask to its owner's address at `RCPT TO`: the mask is the original +/// recipient (`ORCPT`, `rfc822;address`), and must belong to the recipient +/// account (ME-4, ME-9). +pub async fn resolve_original( + data: &Store, + registry: &RegistryStore, + orcpt: Option<&str>, + account_id: u32, +) -> trc::Result> { + let Some(original) = orcpt.map(|orcpt| { + orcpt + .split_once(';') + .map(|(_, address)| address) + .unwrap_or(orcpt) + .trim() + }) else { + return Ok(None); + }; + Ok(resolve_recipient(data, registry, original) + .await? + .filter(|mask| mask.object.account_id.document_id() == account_id)) +} + /// The message as delivered through a mask: an `X-Masked-Email` header /// names the mask, so the user can tell even when it was only BCC'd (ME-9). /// Nothing else in the message changes. diff --git a/crates/jmap/src/inbuxa/masked_email.rs b/crates/jmap/src/inbuxa/masked_email.rs index 22330cd..8094652 100644 --- a/crates/jmap/src/inbuxa/masked_email.rs +++ b/crates/jmap/src/inbuxa/masked_email.rs @@ -170,7 +170,7 @@ impl CreateRefusal { .with_description("emailPrefix must be 1 to 64 characters from a-z, 0-9 and _."), CreateRefusal::DomainNotAllowed => SetError::forbidden() .with_property(domain) - .with_description("The account can't have masked addresses on this domain."), + .with_description("The specified domain is not valid for this account."), CreateRefusal::OverQuota => { SetError::new(jmap_proto::error::set::SetErrorType::OverQuota) .with_description("The account's maxMaskedAddresses limit is reached.") @@ -210,7 +210,15 @@ pub(crate) async fn validate( domain.as_deref(), ) .await? - .map(|_| ObjectResponse::default()) + .map(|_| { + // The address is server-set, so the create response carries it + let mut response = ObjectResponse::default(); + response.object.insert_unchecked( + jmap_tools::Key::Property(Property::Email), + JmapValue::Str(mask.email.clone().into()), + ); + response + }) .map_err(|refusal| { refusal.into_set_error(Property::EmailPrefix, Property::EmailDomain) })) @@ -286,10 +294,8 @@ pub async fn read(server: &Server, id: Id, mask: &mut MaskedEmail) -> trc::Resul /// `x:MaskedEmail/query`, which also filters on `enabled`, `forDomain` and /// text in the address and description (a fork addition). pub(crate) async fn query(mut req: RegistryQueryResponse<'_>) -> trc::Result { - let account_id = req.request.account_id.document_id(); - assert_can_manage(req.server, req.access_token, account_id).await?; - let mut enabled = None; + let mut filter_account = None; let mut for_domain = None; let mut text = None; req.request @@ -306,35 +312,52 @@ pub(crate) async fn query(mut req: RegistryQueryResponse<'_>) -> trc::Result true, + (Property::AccountId, RegistryFilterOp::Equal, serde_json::Value::String(v)) => { + filter_account = ::from_str(&v).ok(); + filter_account.is_some() + } _ => false, })?; req.request .extract_parameters(req.server.core.jmap.query_max_results, Some(Property::Id))?; - let mut ids = ops::of_account( - &req.server.core.storage.data, - req.server.registry(), - account_id, - ) - .await? - .into_iter() - .filter(|mask: &Mask| { - enabled.is_none_or(|e| mask.state.as_upstream_enabled(mask.expired) == e) - && for_domain - .as_deref() - .is_none_or(|d| mask.object.for_domain.as_deref() == Some(d)) - && text.as_deref().is_none_or(|t| { - mask.object.email.to_lowercase().contains(t) - || mask - .object - .description - .as_deref() - .is_some_and(|d| d.to_lowercase().contains(t)) - }) - }) - .map(|mask| mask.id) - .collect::>(); + // ME-19: one account's masks, or every mask for a server-level + // administrator who asks for no account in particular + let data = &req.server.core.storage.data; + let masks = match filter_account { + Some(account) => { + assert_can_manage(req.server, req.access_token, account.document_id()).await?; + ops::of_account(data, req.server.registry(), account.document_id()).await? + } + None if req.access_token.tenant_id().is_none() + && req.access_token.has_permission(Permission::Impersonate) => + { + ops::all(data, req.server.registry()).await? + } + None => { + let account_id = req.request.account_id.document_id(); + assert_can_manage(req.server, req.access_token, account_id).await?; + ops::of_account(data, req.server.registry(), account_id).await? + } + }; + let mut ids = masks + .into_iter() + .filter(|mask: &Mask| { + enabled.is_none_or(|e| mask.state.as_upstream_enabled(mask.expired) == e) + && for_domain + .as_deref() + .is_none_or(|d| mask.object.for_domain.as_deref() == Some(d)) + && text.as_deref().is_none_or(|t| { + mask.object.email.to_lowercase().contains(t) + || mask + .object + .description + .as_deref() + .is_some_and(|d| d.to_lowercase().contains(t)) + }) + }) + .map(|mask| mask.id) + .collect::>(); ids.sort_unstable(); let mut response = QueryResponseBuilder::new( diff --git a/docs/spec/features/masked-email.md b/docs/spec/features/masked-email.md index b6d759d..1a5a0cc 100644 --- a/docs/spec/features/masked-email.md +++ b/docs/spec/features/masked-email.md @@ -303,8 +303,16 @@ upstream files carry hooks marked `inbuxa:`. Acceptance tests 1 to 11 pass as - Creating an account or alias doesn't consult the mask index, so an account could be given an address a mask holds; the account then wins delivery. A random 12-character mask address makes this unlikely. - - The `x:` API's create response carries only the new id, as upstream's - registry responses do. The address is read with `/get`. + - At `RCPT TO` a live mask is rewritten to its owner's address, as + upstream's shared tests expect, so `Delivered-To` names the account + (ME-9). Delivery recognizes the mask from the original recipient + (`ORCPT`) when that mask belongs to the recipient. A sender who knows a + mask can set that `ORCPT` on mail to the owner's own address, which at + most files its own mail to Trash (through a disabled mask) or adds a header + naming the mask. + - Upstream's shared delivery test expected a `.` in a generated address. + ME-13 deliberately differs, so that one assertion checks the fork's + format instead, marked `inbuxa: ME-13`. ## Observed diff --git a/tests/src/system/delivery.rs b/tests/src/system/delivery.rs index 47ee5cb..916dc09 100644 --- a/tests/src/system/delivery.rs +++ b/tests/src/system/delivery.rs @@ -216,8 +216,11 @@ pub async fn test(test: &mut TestServer) { let masked = response.created(0); let masked_random_id = masked.object_id(); let masked_random_email = masked.text_field("email").to_string(); + // inbuxa: ME-13: the fork's addresses never contain a '.' in the local + // part, so they can't be mistaken for upstream's assert!( - masked_random_email.contains(".") && masked_random_email.ends_with("@example.org"), + !masked_random_email.split('@').next().unwrap().contains('.') + && masked_random_email.ends_with("@example.org"), "Unexpected masked email: {masked_random_email}" ); diff --git a/tests/src/system/masked_email.rs b/tests/src/system/masked_email.rs index 5f87412..e6eb130 100644 --- a/tests/src/system/masked_email.rs +++ b/tests/src/system/masked_email.rs @@ -107,6 +107,12 @@ pub async fn test(test: &mut TestServer) { pending_email, "ME-9" ); + // ... and Delivered-To stays the account's real address (observed 3) + assert_eq!( + alice.latest_header("Delivered-To").await.trim(), + "alice@example.org", + "ME-9: Delivered-To" + ); // ME-10: sub-addressing on a mask let (local, domain) = pending_email.split_once('@').unwrap(); @@ -259,6 +265,9 @@ pub async fn test(test: &mut TestServer) { name: "mask-tenant.example.org".to_string(), is_enabled: true, member_tenant_id: Some(t_id), + certificate_management: registry::schema::structs::CertificateManagement::Manual, + dns_management: registry::schema::structs::DnsManagement::Manual, + dkim_management: registry::schema::structs::DkimManagement::Manual, ..Default::default() }) .await; @@ -385,8 +394,14 @@ pub async fn test(test: &mut TestServer) { admin.destroy_account(account).await; } test.wait_for_tasks().await; - admin.registry_destroy(ObjectType::Domain, [t_domain]).await; - admin.registry_destroy(ObjectType::Tenant, [t_id]).await; + admin + .registry_destroy(ObjectType::Domain, [t_domain]) + .await + .assert_destroyed(&[t_domain]); + admin + .registry_destroy(ObjectType::Tenant, [t_id]) + .await + .assert_destroyed(&[t_id]); } /// Acceptance test 12 (compat): masks written before the cutover resolve by