From bcf4a49325a3133ff014b97c84ffd93ece0d62b3 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Fri, 18 Sep 2026 15:20:04 -0700 Subject: [PATCH] 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. --- crates/dav/src/common/acl.rs | 20 ++++++++++++++++ crates/imap/src/op/acl.rs | 22 ++++++++++++++++++ crates/jmap/src/addressbook/set.rs | 4 ++-- crates/jmap/src/api/acl.rs | 37 +++++++++++++++++++++++++----- crates/jmap/src/calendar/set.rs | 4 ++-- crates/jmap/src/file/copy.rs | 2 +- crates/jmap/src/file/set.rs | 4 ++-- crates/jmap/src/mailbox/set.rs | 2 +- 8 files changed, 81 insertions(+), 14 deletions(-) diff --git a/crates/dav/src/common/acl.rs b/crates/dav/src/common/acl.rs index b3a5c79..b345a6d 100644 --- a/crates/dav/src/common/acl.rs +++ b/crates/dav/src/common/acl.rs @@ -137,6 +137,26 @@ impl DavAclHandler for Server { .validate_and_map_aces(access_token, request, collection) .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) { // Refresh ACLs self.refresh_archived_acls(&grants, acls) diff --git a/crates/imap/src/op/acl.rs b/crates/imap/src/op/acl.rs index 8dec273..4413721 100644 --- a/crates/imap/src/op/acl.rs +++ b/crates/imap/src/op/acl.rs @@ -286,6 +286,28 @@ impl Session { .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 let mut mailbox = current_mailbox.inner.clone(); let (op, rights) = arguments diff --git a/crates/jmap/src/addressbook/set.rs b/crates/jmap/src/addressbook/set.rs index 9086aa8..79fccde 100644 --- a/crates/jmap/src/addressbook/set.rs +++ b/crates/jmap/src/addressbook/set.rs @@ -101,7 +101,7 @@ impl AddressBookSet for Server { // Validate ACLs 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()); continue 'create; } @@ -202,7 +202,7 @@ impl AddressBookSet for Server { } } 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()); continue 'update; } diff --git a/crates/jmap/src/api/acl.rs b/crates/jmap/src/api/acl.rs index 2667bf7..b42e945 100644 --- a/crates/jmap/src/api/acl.rs +++ b/crates/jmap/src/api/acl.rs @@ -6,11 +6,11 @@ use common::{Server, auth::AccessToken, sharing::EffectiveAcl}; use jmap_proto::{ - error::set::SetError, + error::set::{SetError, SetErrorType}, object::{JmapRight, JmapSharedObject}, }; 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 types::{ acl::{Acl, AclGrant}, @@ -229,6 +229,7 @@ impl JmapRights { pub trait JmapAcl { fn acl_validate( &self, + account_id: u32, grants: &[AclGrant], ) -> impl Future> + Send; } @@ -239,7 +240,11 @@ pub enum ShareValidationError { } 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 { return Err(ShareValidationError::MaxSharesExceeded( self.core.groupware.max_shares_per_item, @@ -252,8 +257,23 @@ impl JmapAcl for Server { .await .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 { - 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( grant.account_id, ))); @@ -271,8 +291,13 @@ impl From for SetError { .with_description(format!( "Maximum number of shares per item exceeded (max: {max})" )), - ShareValidationError::InvalidAccountId(id) => SetError::invalid_properties() - .with_description(format!("Account id {id} is invalid.")), + // inbuxa: MT-3: the same error whether the account is missing or + // 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.")) + } } } } diff --git a/crates/jmap/src/calendar/set.rs b/crates/jmap/src/calendar/set.rs index f41f10b..733510a 100644 --- a/crates/jmap/src/calendar/set.rs +++ b/crates/jmap/src/calendar/set.rs @@ -105,7 +105,7 @@ impl CalendarSet for Server { // Validate ACLs 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()); continue 'create; } @@ -206,7 +206,7 @@ impl CalendarSet for Server { } } 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()); continue 'update; } diff --git a/crates/jmap/src/file/copy.rs b/crates/jmap/src/file/copy.rs index e1dd511..d29821c 100644 --- a/crates/jmap/src/file/copy.rs +++ b/crates/jmap/src/file/copy.rs @@ -370,7 +370,7 @@ impl FileNodeCopy for Server { } 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()); continue 'create; } diff --git a/crates/jmap/src/file/set.rs b/crates/jmap/src/file/set.rs index 0b3d282..576fdbf 100644 --- a/crates/jmap/src/file/set.rs +++ b/crates/jmap/src/file/set.rs @@ -270,7 +270,7 @@ impl FileNodeSet for Server { // Validate ACLs 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()); continue 'create; } @@ -502,7 +502,7 @@ impl FileNodeSet for Server { } } 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()); continue 'update; } diff --git a/crates/jmap/src/mailbox/set.rs b/crates/jmap/src/mailbox/set.rs index 430c469..f37dd06 100644 --- a/crates/jmap/src/mailbox/set.rs +++ b/crates/jmap/src/mailbox/set.rs @@ -612,7 +612,7 @@ impl MailboxSet for Server { let current = update.map(|(_, current)| current); if has_acl_changes { 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())); }