From 58d2804278ec736b0eda512dc1f9945218099aa1 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Mon, 5 Oct 2026 15:03:09 -0700 Subject: [PATCH] Don't let a group's members share its calendars, address books or files #146 stopped a group's members sharing its mailboxes on. The same shortcut lets them through everywhere else a group owns things: a member counts as the account's owner, so Calendar/set, AddressBook/set and FileNode/set skip the share check, and so does the WebDAV ACL method. Who has what a group owns is decided by who is in the group. For a member through a group only (is_group_member_only): - Calendar/set, AddressBook/set and FileNode/set refuse a shareWith change as forbidden, on create and update; for files at the top of the account too, not only inside a folder; - the DAV ACL method answers 403 on the group's calendars, address books and files; - myRights reports mayShare false (JmapRights::owner_rights), and the DAV current-user-privilege-set leaves out all and write-acl. Reading who something is shared with is unchanged, as in JMAP. Tests: a new jmap::group_share module has a member create with a share, create without one (and check myRights), share afterwards, and an outsider reach each kind; the WebDAV ACL test has a member try the ACL method on the group's folders; the IMAP ACL test now checks #146's SETACL refusal, which had no test of its own. jmap_tests, webdav_tests and imap_tests pass (RocksDB). specs/multi-account.md MA-D0. --- crates/dav/src/common/acl.rs | 12 ++- crates/jmap/src/addressbook/get.rs | 4 +- crates/jmap/src/addressbook/set.rs | 16 ++++ crates/jmap/src/api/acl.rs | 15 ++++ crates/jmap/src/calendar/get.rs | 4 +- crates/jmap/src/calendar/set.rs | 16 ++++ crates/jmap/src/file/get.rs | 4 +- crates/jmap/src/file/set.rs | 18 ++++ tests/src/imap/acl.rs | 11 +++ tests/src/jmap/group_share.rs | 131 +++++++++++++++++++++++++++++ tests/src/jmap/mod.rs | 4 + tests/src/webdav/acl.rs | 36 ++++++++ 12 files changed, 267 insertions(+), 4 deletions(-) create mode 100644 tests/src/jmap/group_share.rs diff --git a/crates/dav/src/common/acl.rs b/crates/dav/src/common/acl.rs index bd9c8c4..1444fdc 100644 --- a/crates/dav/src/common/acl.rs +++ b/crates/dav/src/common/acl.rs @@ -133,6 +133,10 @@ impl DavAclHandler for Server { { return Err(DavError::Code(StatusCode::FORBIDDEN)); } + // inbuxa: MA-D0: a group's members don't share what it owns on. + if access_token.is_group_member_only(account_id) { + return Err(DavError::Code(StatusCode::FORBIDDEN)); + } // Validate ACEs let grants = self @@ -565,7 +569,13 @@ impl Privileges for AccessToken { grants: &ArchivedVec, is_calendar: bool, ) -> Vec { - if self.is_member(account_id) { + if self.is_group_member_only(account_id) { + // inbuxa: MA-D0: everything but sharing it on. + Privilege::all(is_calendar) + .into_iter() + .filter(|privilege| !matches!(privilege, Privilege::All | Privilege::WriteAcl)) + .collect() + } else if self.is_member(account_id) { Privilege::all(is_calendar) } else { current_user_privilege_set(grants.effective_acl(self)) diff --git a/crates/jmap/src/addressbook/get.rs b/crates/jmap/src/addressbook/get.rs index 74624b6..f078f48 100644 --- a/crates/jmap/src/addressbook/get.rs +++ b/crates/jmap/src/addressbook/get.rs @@ -2,6 +2,8 @@ * SPDX-FileCopyrightText: 2020 Stalwart Labs LLC * * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL + * + * Modified by Coffey Labs in 2026 for INBUXA. */ use crate::{api::acl::JmapRights, changes::state::JmapCacheState}; @@ -180,7 +182,7 @@ impl AddressBookGet for Server { address_book.acls.effective_acl(access_token), ) } else { - JmapRights::all_rights::() + JmapRights::owner_rights::(access_token, account_id) }, ); } diff --git a/crates/jmap/src/addressbook/set.rs b/crates/jmap/src/addressbook/set.rs index f40a007..12578d2 100644 --- a/crates/jmap/src/addressbook/set.rs +++ b/crates/jmap/src/addressbook/set.rs @@ -101,6 +101,14 @@ impl AddressBookSet for Server { continue 'create; } + // inbuxa: MA-D0: a group's members don't share what it owns on. + if !address_book.acls.is_empty() && access_token.is_group_member_only(account_id) { + response.not_created.append( + id, + SetError::forbidden().with_description("This belongs to a group. Only an administrator can change who has it."), + ); + continue 'create; + } // Validate ACLs if !address_book.acls.is_empty() { if let Err(err) = self.acl_validate(account_id, &address_book.acls).await { @@ -203,6 +211,14 @@ impl AddressBookSet for Server { continue 'update; } } + // inbuxa: MA-D0: a group's members don't share what it owns on. + if has_acl_changes && access_token.is_group_member_only(account_id) { + response.not_updated.append( + id, + SetError::forbidden().with_description("This belongs to a group. Only an administrator can change who has it."), + ); + continue 'update; + } if has_acl_changes { if let Err(err) = self.acl_validate(account_id, &new_address_book.acls).await { response.not_updated.append(id, err.into()); diff --git a/crates/jmap/src/api/acl.rs b/crates/jmap/src/api/acl.rs index 91b561a..73f389e 100644 --- a/crates/jmap/src/api/acl.rs +++ b/crates/jmap/src/api/acl.rs @@ -187,6 +187,21 @@ impl JmapRights { Value::Object(obj) } + /// inbuxa: MA-D0: an owner's rights, which for a group's member are + /// everything but sharing it on. + pub fn owner_rights( + access_token: &AccessToken, + account_id: u32, + ) -> Value<'static, T::Property, T::Element> { + if access_token.is_group_member_only(account_id) { + let mut acl = Bitmap::::all(); + acl.remove(Acl::Share); + Self::rights::(acl) + } else { + Self::all_rights::() + } + } + pub fn rights( acls: Bitmap, ) -> Value<'static, T::Property, T::Element> { diff --git a/crates/jmap/src/calendar/get.rs b/crates/jmap/src/calendar/get.rs index 76c36c9..868dd4d 100644 --- a/crates/jmap/src/calendar/get.rs +++ b/crates/jmap/src/calendar/get.rs @@ -2,6 +2,8 @@ * SPDX-FileCopyrightText: 2020 Stalwart Labs LLC * * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL + * + * Modified by Coffey Labs in 2026 for INBUXA. */ use crate::{api::acl::JmapRights, calendar::Availability, changes::state::JmapCacheState}; @@ -253,7 +255,7 @@ impl CalendarGet for Server { calendar.acls.effective_acl(access_token), ) } else { - JmapRights::all_rights::() + JmapRights::owner_rights::(access_token, account_id) }, ); } diff --git a/crates/jmap/src/calendar/set.rs b/crates/jmap/src/calendar/set.rs index c6ac11a..533afe9 100644 --- a/crates/jmap/src/calendar/set.rs +++ b/crates/jmap/src/calendar/set.rs @@ -105,6 +105,14 @@ impl CalendarSet for Server { continue 'create; } + // inbuxa: MA-D0: a group's members don't share what it owns on. + if !calendar.acls.is_empty() && access_token.is_group_member_only(account_id) { + response.not_created.append( + id, + SetError::forbidden().with_description("This belongs to a group. Only an administrator can change who has it."), + ); + continue 'create; + } // Validate ACLs if !calendar.acls.is_empty() { if let Err(err) = self.acl_validate(account_id, &calendar.acls).await { @@ -207,6 +215,14 @@ impl CalendarSet for Server { continue 'update; } } + // inbuxa: MA-D0: a group's members don't share what it owns on. + if has_acl_changes && access_token.is_group_member_only(account_id) { + response.not_updated.append( + id, + SetError::forbidden().with_description("This belongs to a group. Only an administrator can change who has it."), + ); + continue 'update; + } if has_acl_changes { if let Err(err) = self.acl_validate(account_id, &new_calendar.acls).await { response.not_updated.append(id, err.into()); diff --git a/crates/jmap/src/file/get.rs b/crates/jmap/src/file/get.rs index 6846b80..6f3c0e3 100644 --- a/crates/jmap/src/file/get.rs +++ b/crates/jmap/src/file/get.rs @@ -2,6 +2,8 @@ * SPDX-FileCopyrightText: 2020 Stalwart Labs LLC * * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL + * + * Modified by Coffey Labs in 2026 for INBUXA. */ use crate::{api::acl::JmapRights, changes::state::JmapCacheState}; @@ -172,7 +174,7 @@ impl FileNodeGet for Server { file_node.acls.effective_acl(access_token), ) } else { - JmapRights::all_rights::() + JmapRights::owner_rights::(access_token, account_id) }, ); } diff --git a/crates/jmap/src/file/set.rs b/crates/jmap/src/file/set.rs index 428eb55..723a697 100644 --- a/crates/jmap/src/file/set.rs +++ b/crates/jmap/src/file/set.rs @@ -250,6 +250,16 @@ impl FileNodeSet for Server { }, }; + // inbuxa: MA-D0: a group's members don't share what it owns on, + // at the top of its files as anywhere else + if has_acl_changes && access_token.is_group_member_only(account_id) { + response.not_created.append( + id, + SetError::forbidden().with_description("This belongs to a group. Only an administrator can change who has it."), + ); + continue 'create; + } + // Inherit ACLs from parent if file_node.parent_id > 0 { let parent_id = file_node.parent_id - 1; @@ -509,6 +519,14 @@ impl FileNodeSet for Server { continue 'update; } } + // inbuxa: MA-D0: a group's members don't share what it owns on. + if has_acl_changes && access_token.is_group_member_only(account_id) { + response.not_updated.append( + id, + SetError::forbidden().with_description("This belongs to a group. Only an administrator can change who has it."), + ); + continue 'update; + } if has_acl_changes { if let Err(err) = self.acl_validate(account_id, &new_file_node.acls).await { response.not_updated.append(id, err.into()); diff --git a/tests/src/imap/acl.rs b/tests/src/imap/acl.rs index 33103ad..dcc5805 100644 --- a/tests/src/imap/acl.rs +++ b/tests/src/imap/acl.rs @@ -2,6 +2,8 @@ * SPDX-FileCopyrightText: 2020 Stalwart Labs LLC * * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL + * + * Modified by Coffey Labs in 2026 for INBUXA. */ use super::{AssertResult, ImapConnection, Type, append::assert_append_message}; @@ -69,6 +71,15 @@ pub async fn test( .await; imap_jane.assert_read(Type::Tagged, ResponseType::Ok).await; + // inbuxa: MA-D0: but she can't share the group's mailbox on + imap_jane + .send("SETACL \"Shared Folders/support@example.com/INBOX\" jdoe@example.com lr") + .await; + imap_jane + .assert_read(Type::Tagged, ResponseType::No) + .await + .assert_contains("NOPERM"); + // John should have no shared folders imap_john.send("LIST \"\" \"*\"").await; imap_john diff --git a/tests/src/jmap/group_share.rs b/tests/src/jmap/group_share.rs new file mode 100644 index 0000000..8eb1edf --- /dev/null +++ b/tests/src/jmap/group_share.rs @@ -0,0 +1,131 @@ +/* + * SPDX-FileCopyrightText: 2026 Coffey Labs + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +//! MA-D0 (specs/multi-account.md): a group's members have its calendars, +//! address books and files, but can't share them on. Who is in a group is +//! an administrator's decision. Mailboxes are checked in `mail::acl`. + +use crate::utils::{jmap::JmapUtils, server::TestServer}; +use jmap_proto::request::method::MethodObject; +use registry::schema::prelude::ObjectType; +use serde_json::json; + +pub async fn test(test: &TestServer) { + println!("Running group sharing tests..."); + let admin = test.account("admin@example.com"); + let sales = test.account("sales@example.com"); + let bill = test.account("bill@example.com"); + let robert = test.account("robert@example.com"); + let robert_id = robert.id_string().to_string(); + + // Bill joins the group; Robert stays outside it + admin + .registry_update_object( + ObjectType::Account, + bill.id(), + json!({"memberGroupIds": {sales.id_string(): true}}), + ) + .await; + + // Each kind's own name for "may read" + for (object, read) in [ + (MethodObject::Calendar, "mayReadItems"), + (MethodObject::AddressBook, "mayRead"), + (MethodObject::FileNode, "mayRead"), + ] { + // Made with a share: refused + let response = bill + .jmap_create_account( + sales, + object, + [json!({ + "name": "Shared on", + "shareWith": {&robert_id: {read: true}} + })], + Vec::<(&str, &str)>::new(), + ) + .await; + assert_eq!( + response.pointer("/methodResponses/0/1/notCreated/i0/type"), + Some(&json!("forbidden")), + "MA-D0: {object} created with a share: {:?}", + response.pointer("/methodResponses/0") + ); + + // Made without one: fine, and it says it can't be shared + let id = bill + .jmap_create_account( + sales, + object, + [json!({"name": "The group's"})], + Vec::<(&str, &str)>::new(), + ) + .await + .created(0) + .id() + .to_string(); + let rights = bill + .jmap_get_account(sales, object, ["myRights"], [id.as_str()]) + .await + .list()[0]["myRights"] + .clone(); + assert_eq!(rights["mayShare"], false, "MA-D0: {object} myRights {rights}"); + assert_eq!(rights["mayDelete"], true, "MA-D0: {object} myRights {rights}"); + + // Shared afterwards: refused + let response = bill + .jmap_update_account( + sales, + object, + [( + &id, + json!({format!("shareWith/{robert_id}"): {read: true}}), + )], + Vec::<(&str, &str)>::new(), + ) + .await; + assert_eq!( + response.pointer(&format!("/methodResponses/0/1/notUpdated/{id}/type")), + Some(&json!("forbidden")), + "MA-D0: {object} shared on: {:?}", + response.pointer("/methodResponses/0") + ); + + // Robert still has nothing + assert_eq!( + robert + .jmap_get_account(sales, object, Vec::<&str>::new(), [id.as_str()]) + .await + .method_response() + .typ(), + "forbidden", + "MA-D0: {object} reached from outside" + ); + + bill.jmap_destroy_account(sales, object, [id.as_str()], Vec::<(&str, &str)>::new()) + .await; + } + + // Reaching the group's calendars and address books made its defaults + let sales_id = sales.id_string(); + bill.jmap_method_calls(json!([ + ["Calendar/get", {"accountId": sales_id, "ids": (), "properties": ["id"]}, "c"], + ["Calendar/set", {"accountId": sales_id, "onDestroyRemoveEvents": true, + "#destroy": {"resultOf": "c", "name": "Calendar/get", "path": "/list/*/id"}}, "cd"], + ["AddressBook/get", {"accountId": sales_id, "ids": (), "properties": ["id"]}, "a"], + ["AddressBook/set", {"accountId": sales_id, "onDestroyRemoveContents": true, + "#destroy": {"resultOf": "a", "name": "AddressBook/get", "path": "/list/*/id"}}, "ad"] + ])) + .await; + + admin + .registry_update_object( + ObjectType::Account, + bill.id(), + json!({"memberGroupIds": {sales.id_string(): false}}), + ) + .await; +} diff --git a/tests/src/jmap/mod.rs b/tests/src/jmap/mod.rs index aa0ec35..76cc5ee 100644 --- a/tests/src/jmap/mod.rs +++ b/tests/src/jmap/mod.rs @@ -2,6 +2,8 @@ * SPDX-FileCopyrightText: 2020 Stalwart Labs LLC * * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL + * + * Modified by Coffey Labs in 2026 for INBUXA. */ use crate::utils::server::TestServerBuilder; @@ -22,6 +24,7 @@ pub mod compliance; pub mod contacts; pub mod core; pub mod files; +pub mod group_share; pub mod mail; pub mod principal; @@ -219,6 +222,7 @@ pub async fn jmap_tests() { calendar::identity::test(&test).await; calendar::acl::test(&test).await; + group_share::test(&test).await; principal::get::test(&test).await; principal::availability::test(&test).await; diff --git a/tests/src/webdav/acl.rs b/tests/src/webdav/acl.rs index 13498c1..08c1a77 100644 --- a/tests/src/webdav/acl.rs +++ b/tests/src/webdav/acl.rs @@ -2,6 +2,8 @@ * SPDX-FileCopyrightText: 2020 Stalwart Labs LLC * * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL + * + * Modified by Coffey Labs in 2026 for INBUXA. */ use crate::utils::{server::TestServer, webdav::GenerateTestDavResource}; @@ -407,6 +409,40 @@ pub async fn test(test: &TestServer) { .with_status(StatusCode::NO_CONTENT); } + // inbuxa: MA-D0: Jane, a member of the Support group, can make a folder in + // the group's account but can't share it on + let member_client = test.account("jane@example.com").webdav_client(); + let john_principal = format!( + "{}/john%40example.com/", + DavResourceName::Principal.base_path() + ); + for resource_type in [ + DavResourceName::File, + DavResourceName::Cal, + DavResourceName::Card, + ] { + let group_folder = format!( + "{}/support%40example.com/group-folder/", + resource_type.base_path() + ); + member_client + .request("MKCOL", &group_folder, "") + .await + .with_status(StatusCode::CREATED); + member_client + .acl(&group_folder, john_principal.as_str(), ["read"]) + .await + .with_status(StatusCode::FORBIDDEN); + member_client + .request("DELETE", &group_folder, "") + .await + .with_status(StatusCode::NO_CONTENT); + } + // Reaching the group's calendars and address books made its defaults + member_client + .delete_default_containers_by_account("support@example.com") + .await; + sharee_client.delete_default_containers().await; owner_client.delete_default_containers().await; test.assert_is_empty().await;