Multi-tenancy: sharing grants stay within the owner's tenant (MT-1, MT-3)

JMAP shareWith refuses a grantee outside the owner's tenant, or missing, with
invalidForeignKey naming the account. WebDAV ACL answers AllowedPrincipal and
IMAP SETACL answers as for an unknown account.
This commit is contained in:
2026-09-18 15:20:04 -07:00
parent cea8b1593b
commit bcf4a49325
8 changed files with 81 additions and 14 deletions
+20
View File
@@ -137,6 +137,26 @@ impl DavAclHandler for Server {
.validate_and_map_aces(access_token, request, collection) .validate_and_map_aces(access_token, request, collection)
.await?; .await?;
// inbuxa: MT-3: grants stay within the owner's tenant
let tenant_id = self
.try_account(account_id)
.await
.caused_by(trc::location!())?
.and_then(|owner| owner.id_tenant);
for grant in &grants {
if self
.try_account(grant.account_id)
.await
.caused_by(trc::location!())?
.is_none_or(|grantee| grantee.id_tenant != tenant_id)
{
return Err(DavError::Condition(DavErrorCondition::new(
StatusCode::FORBIDDEN,
BaseCondition::AllowedPrincipal,
)));
}
}
if grants.len() != acls.len() || acls.iter().zip(grants.iter()).any(|(a, b)| a != b) { if grants.len() != acls.len() || acls.iter().zip(grants.iter()).any(|(a, b)| a != b) {
// Refresh ACLs // Refresh ACLs
self.refresh_archived_acls(&grants, acls) self.refresh_archived_acls(&grants, acls)
+22
View File
@@ -286,6 +286,28 @@ impl<T: SessionStream> Session<T> {
.caused_by(trc::location!()) .caused_by(trc::location!())
})?; })?;
// inbuxa: MT-3: grants stay within the owner's tenant, refused
// as if the account didn't exist
let owner_tenant = data
.server
.try_account(mailbox_id.account_id)
.await
.imap_ctx(&arguments.tag, trc::location!())?
.and_then(|owner| owner.id_tenant);
if data
.server
.try_account(acl_account_id)
.await
.imap_ctx(&arguments.tag, trc::location!())?
.is_none_or(|grantee| grantee.id_tenant != owner_tenant)
{
return Err(trc::ImapEvent::Error
.into_err()
.details("Account does not exist")
.id(arguments.tag.to_string())
.caused_by(trc::location!()));
}
// Prepare changes // Prepare changes
let mut mailbox = current_mailbox.inner.clone(); let mut mailbox = current_mailbox.inner.clone();
let (op, rights) = arguments let (op, rights) = arguments
+2 -2
View File
@@ -101,7 +101,7 @@ impl AddressBookSet for Server {
// Validate ACLs // Validate ACLs
if !address_book.acls.is_empty() { if !address_book.acls.is_empty() {
if let Err(err) = self.acl_validate(&address_book.acls).await { if let Err(err) = self.acl_validate(account_id, &address_book.acls).await {
response.not_created.append(id, err.into()); response.not_created.append(id, err.into());
continue 'create; continue 'create;
} }
@@ -202,7 +202,7 @@ impl AddressBookSet for Server {
} }
} }
if has_acl_changes { if has_acl_changes {
if let Err(err) = self.acl_validate(&new_address_book.acls).await { if let Err(err) = self.acl_validate(account_id, &new_address_book.acls).await {
response.not_updated.append(id, err.into()); response.not_updated.append(id, err.into());
continue 'update; continue 'update;
} }
+31 -6
View File
@@ -6,11 +6,11 @@
use common::{Server, auth::AccessToken, sharing::EffectiveAcl}; use common::{Server, auth::AccessToken, sharing::EffectiveAcl};
use jmap_proto::{ use jmap_proto::{
error::set::SetError, error::set::{SetError, SetErrorType},
object::{JmapRight, JmapSharedObject}, object::{JmapRight, JmapSharedObject},
}; };
use jmap_tools::{JsonPointerIter, Key, Map, Property, Value}; use jmap_tools::{JsonPointerIter, Key, Map, Property, Value};
use registry::schema::prelude::ObjectType; use registry::{schema::prelude::ObjectType, types::id::ObjectId};
use store::{registry::RegistryQuery, roaring::RoaringBitmap}; use store::{registry::RegistryQuery, roaring::RoaringBitmap};
use types::{ use types::{
acl::{Acl, AclGrant}, acl::{Acl, AclGrant},
@@ -229,6 +229,7 @@ impl JmapRights {
pub trait JmapAcl { pub trait JmapAcl {
fn acl_validate( fn acl_validate(
&self, &self,
account_id: u32,
grants: &[AclGrant], grants: &[AclGrant],
) -> impl Future<Output = Result<(), ShareValidationError>> + Send; ) -> impl Future<Output = Result<(), ShareValidationError>> + Send;
} }
@@ -239,7 +240,11 @@ pub enum ShareValidationError {
} }
impl JmapAcl for Server { impl JmapAcl for Server {
async fn acl_validate(&self, grants: &[AclGrant]) -> Result<(), ShareValidationError> { async fn acl_validate(
&self,
account_id: u32,
grants: &[AclGrant],
) -> Result<(), ShareValidationError> {
if grants.len() > self.core.groupware.max_shares_per_item { if grants.len() > self.core.groupware.max_shares_per_item {
return Err(ShareValidationError::MaxSharesExceeded( return Err(ShareValidationError::MaxSharesExceeded(
self.core.groupware.max_shares_per_item, self.core.groupware.max_shares_per_item,
@@ -252,8 +257,23 @@ impl JmapAcl for Server {
.await .await
.unwrap_or_default(); .unwrap_or_default();
// inbuxa: MT-3: grants stay within the owner's tenant
let tenant_id = self
.try_account(account_id)
.await
.ok()
.flatten()
.and_then(|owner| owner.id_tenant);
for grant in grants { for grant in grants {
if !principal_ids.contains(grant.account_id) { if !principal_ids.contains(grant.account_id)
|| self
.try_account(grant.account_id)
.await
.ok()
.flatten()
.is_none_or(|grantee| grantee.id_tenant != tenant_id)
{
return Err(ShareValidationError::InvalidAccountId(Id::from( return Err(ShareValidationError::InvalidAccountId(Id::from(
grant.account_id, grant.account_id,
))); )));
@@ -271,8 +291,13 @@ impl<T: Property> From<ShareValidationError> for SetError<T> {
.with_description(format!( .with_description(format!(
"Maximum number of shares per item exceeded (max: {max})" "Maximum number of shares per item exceeded (max: {max})"
)), )),
ShareValidationError::InvalidAccountId(id) => SetError::invalid_properties() // inbuxa: MT-3: the same error whether the account is missing or
.with_description(format!("Account id {id} is invalid.")), // in another tenant, so it never confirms the account exists
ShareValidationError::InvalidAccountId(id) => {
SetError::new(SetErrorType::InvalidForeignKey)
.with_object_id(ObjectId::new(ObjectType::Account, id))
.with_description(format!("Account id {id} is invalid."))
}
} }
} }
} }
+2 -2
View File
@@ -105,7 +105,7 @@ impl CalendarSet for Server {
// Validate ACLs // Validate ACLs
if !calendar.acls.is_empty() { if !calendar.acls.is_empty() {
if let Err(err) = self.acl_validate(&calendar.acls).await { if let Err(err) = self.acl_validate(account_id, &calendar.acls).await {
response.not_created.append(id, err.into()); response.not_created.append(id, err.into());
continue 'create; continue 'create;
} }
@@ -206,7 +206,7 @@ impl CalendarSet for Server {
} }
} }
if has_acl_changes { if has_acl_changes {
if let Err(err) = self.acl_validate(&new_calendar.acls).await { if let Err(err) = self.acl_validate(account_id, &new_calendar.acls).await {
response.not_updated.append(id, err.into()); response.not_updated.append(id, err.into());
continue 'update; continue 'update;
} }
+1 -1
View File
@@ -370,7 +370,7 @@ impl FileNodeCopy for Server {
} }
if !file_node.acls.is_empty() { if !file_node.acls.is_empty() {
if let Err(err) = self.acl_validate(&file_node.acls).await { if let Err(err) = self.acl_validate(account_id, &file_node.acls).await {
response.not_created.append(id, err.into()); response.not_created.append(id, err.into());
continue 'create; continue 'create;
} }
+2 -2
View File
@@ -270,7 +270,7 @@ impl FileNodeSet for Server {
// Validate ACLs // Validate ACLs
if !file_node.acls.is_empty() { if !file_node.acls.is_empty() {
if let Err(err) = self.acl_validate(&file_node.acls).await { if let Err(err) = self.acl_validate(account_id, &file_node.acls).await {
response.not_created.append(id, err.into()); response.not_created.append(id, err.into());
continue 'create; continue 'create;
} }
@@ -502,7 +502,7 @@ impl FileNodeSet for Server {
} }
} }
if has_acl_changes { if has_acl_changes {
if let Err(err) = self.acl_validate(&new_file_node.acls).await { if let Err(err) = self.acl_validate(account_id, &new_file_node.acls).await {
response.not_updated.append(id, err.into()); response.not_updated.append(id, err.into());
continue 'update; continue 'update;
} }
+1 -1
View File
@@ -612,7 +612,7 @@ impl MailboxSet for Server {
let current = update.map(|(_, current)| current); let current = update.map(|(_, current)| current);
if has_acl_changes { if has_acl_changes {
if !changes.acls.is_empty() if !changes.acls.is_empty()
&& let Err(err) = self.acl_validate(&changes.acls).await && let Err(err) = self.acl_validate(ctx.account_id, &changes.acls).await
{ {
return Ok(Err(err.into())); return Ok(Err(err.into()));
} }