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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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<Vec<Mask>> {
|
||||
let mut masks = Vec::new();
|
||||
for id in registry
|
||||
.query::<Vec<Id>>(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<Option<Mask>> {
|
||||
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.
|
||||
|
||||
@@ -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<QueryResponseBuilder> {
|
||||
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<Que
|
||||
text = Some(v.to_lowercase());
|
||||
true
|
||||
}
|
||||
(Property::AccountId, _, _) => true,
|
||||
(Property::AccountId, RegistryFilterOp::Equal, serde_json::Value::String(v)) => {
|
||||
filter_account = <Id as std::str::FromStr>::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::<Vec<_>>();
|
||||
// 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::<Vec<_>>();
|
||||
ids.sort_unstable();
|
||||
|
||||
let mut response = QueryResponseBuilder::new(
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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}"
|
||||
);
|
||||
|
||||
|
||||
@@ -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(),
|
||||
"[email protected]",
|
||||
"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
|
||||
|
||||
Reference in New Issue
Block a user