From 4e3c0582b81ecaee3fa6d2e67482dba1282ce1b9 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Sun, 1 Nov 2026 16:23:31 -0700 Subject: [PATCH] Make "Allow Directory Queries" apply to regular users With the setting off, Principal/query, Principal/get, Principal/getAvailability and DAV principal-property-search still answered anyone holding the matching permission, and every default user role holds those. So any user could list the accounts in their tenant. Off now means off, except for accounts holding sysAccountQuery, which can list accounts anyway. Principal/get and getAvailability still answer for the account itself, its groups and whoever shared with it. /api/account reports the same thing the server now does. principal-match and acl-principal-prop-set aren't directory reads, so the setting doesn't apply to them. Their checks never fired and are gone. --- crates/common/src/auth/permissions.rs | 9 ++ crates/dav/src/request.rs | 34 ++---- crates/http/src/auth/permissions.rs | 2 +- crates/jmap/src/principal/availability.rs | 22 ++-- crates/jmap/src/principal/get.rs | 20 ++-- crates/jmap/src/principal/query.rs | 8 +- tests/src/jmap/mod.rs | 2 + tests/src/jmap/principal/directory_off.rs | 139 ++++++++++++++++++++++ tests/src/jmap/principal/mod.rs | 3 + tests/src/webdav/principals.rs | 69 ++++++++++- 10 files changed, 256 insertions(+), 52 deletions(-) create mode 100644 tests/src/jmap/principal/directory_off.rs diff --git a/crates/common/src/auth/permissions.rs b/crates/common/src/auth/permissions.rs index 79cbe49b..4d46efc8 100644 --- a/crates/common/src/auth/permissions.rs +++ b/crates/common/src/auth/permissions.rs @@ -23,6 +23,15 @@ use types::id::Id; use utils::map::vec_map::VecMap; impl Server { + /// inbuxa: whether this account may look through the server's accounts. + /// `allowDirectoryQueries` decides, except for those who can list accounts + /// anyway. The `jmapPrincipal*` and `davPrincipal*` permissions don't count + /// here: every default user role holds them. + pub fn can_read_directory(&self, access_token: &AccessToken) -> bool { + self.core.groupware.allow_directory_query + || access_token.has_permission(Permission::SysAccountQuery) + } + pub async fn add_role_permissions( &self, mut base_permissions: PermissionsGroup, diff --git a/crates/dav/src/request.rs b/crates/dav/src/request.rs index 87bf87e2..c90435e3 100644 --- a/crates/dav/src/request.rs +++ b/crates/dav/src/request.rs @@ -216,18 +216,9 @@ impl DavRequestDispatcher for Server { } } Report::AclPrincipalPropSet(report) => { - // Validate permissions - if !self.core.groupware.allow_directory_query - && !access_token.has_permission(Permission::DavPrincipalAcl) - { - return Err(DavError::Condition( - DavErrorCondition::new( - StatusCode::FORBIDDEN, - BaseCondition::NeedPrivileges(List(Default::default())), - ) - .with_details("The administrator has disabled directory queries."), - )); - } + // inbuxa: not a directory read, so allowDirectoryQueries + // doesn't apply: the item's owner or a sharer, and only + // the accounts already on its ACL. let access_token = access_token.assert_has_permission(Permission::DavPrincipalAcl)?; @@ -235,18 +226,9 @@ impl DavRequestDispatcher for Server { .await } Report::PrincipalMatch(report) => { - // Validate permissions - if !self.core.groupware.allow_directory_query - && !access_token.has_permission(Permission::DavPrincipalMatch) - { - return Err(DavError::Condition( - DavErrorCondition::new( - StatusCode::FORBIDDEN, - BaseCondition::NeedPrivileges(List(Default::default())), - ) - .with_details("The administrator has disabled directory queries."), - )); - } + // inbuxa: not a directory read, so allowDirectoryQueries + // doesn't apply: it answers with the caller's own accounts, + // and clients use it for discovery. let access_token = access_token.assert_has_permission(Permission::DavPrincipalMatch)?; @@ -256,9 +238,7 @@ impl DavRequestDispatcher for Server { Report::PrincipalPropertySearch(report) => { if resource == DavResourceName::Principal { // Validate permissions - if !self.core.groupware.allow_directory_query - && !access_token.has_permission(Permission::DavPrincipalSearch) - { + if !self.can_read_directory(&access_token) { return Err(DavError::Condition( DavErrorCondition::new( StatusCode::FORBIDDEN, diff --git a/crates/http/src/auth/permissions.rs b/crates/http/src/auth/permissions.rs index 14a7e151..675080ee 100644 --- a/crates/http/src/auth/permissions.rs +++ b/crates/http/src/auth/permissions.rs @@ -77,7 +77,7 @@ impl AccountApiHandler for Server { permissions.clear(p.to_id() as usize); } - if !self.core.groupware.allow_directory_query { + if !self.can_read_directory(access_token) { for p in [ Permission::JmapPrincipalQuery, Permission::JmapPrincipalQueryChanges, diff --git a/crates/jmap/src/principal/availability.rs b/crates/jmap/src/principal/availability.rs index 31771347..a9a710a1 100644 --- a/crates/jmap/src/principal/availability.rs +++ b/crates/jmap/src/principal/availability.rs @@ -34,7 +34,6 @@ use jmap_proto::{ types::date::UTCDate, }; use jmap_tools::{Key, Map, Value}; -use registry::schema::enums::Permission; use std::{collections::hash_map::Entry, future::Future}; use store::{ ValueKey, @@ -64,20 +63,25 @@ impl PrincipalGetAvailability for Server { request: GetAvailabilityRequest, access_token: &AccessToken, ) -> trc::Result { - if !self.core.groupware.allow_directory_query - && !access_token.has_permission(Permission::JmapPrincipalGetAvailability) - { - return Err(trc::JmapEvent::Forbidden - .into_err() - .details("The administrator has disabled directory queries.".to_string())); - } - // Process parameters if !request.id.is_valid() { return Err(trc::JmapEvent::InvalidArguments .into_err() .details("Missing principal id")); } + + // inbuxa: with directory queries off, only the accounts this one already + // has to do with. Same answer whether the id exists or not. + if !self.can_read_directory(access_token) + && !access_token + .all_ids() + .any(|id| id == request.id.document_id()) + { + return Err(trc::JmapEvent::Forbidden + .into_err() + .details("The administrator has disabled directory queries.".to_string())); + } + let properties = request .event_properties .map(|props| props.into_valid().collect::>()) diff --git a/crates/jmap/src/principal/get.rs b/crates/jmap/src/principal/get.rs index 0071e3da..14a34230 100644 --- a/crates/jmap/src/principal/get.rs +++ b/crates/jmap/src/principal/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 common::{Server, auth::AccessToken}; @@ -12,7 +14,7 @@ use jmap_proto::{ types::state::State, }; use jmap_tools::{Key, Map, Value}; -use registry::schema::prelude::{ObjectType, Permission}; +use registry::schema::prelude::ObjectType; use std::future::Future; use store::{registry::RegistryQuery, roaring::RoaringBitmap}; use trc::AddContext; @@ -31,14 +33,6 @@ impl PrincipalGet for Server { mut request: GetRequest, access_token: &AccessToken, ) -> trc::Result> { - if !self.core.groupware.allow_directory_query - && !access_token.has_permission(Permission::JmapPrincipalGet) - { - return Err(trc::JmapEvent::Forbidden - .into_err() - .details("The administrator has disabled directory queries.".to_string())); - } - let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let properties = request.unwrap_properties(&[ PrincipalProperty::Id, @@ -49,7 +43,7 @@ impl PrincipalGet for Server { ]); // Return all principals - let principal_ids = self + let mut principal_ids = self .registry() .query::( RegistryQuery::new(ObjectType::Account).with_tenant(access_token.tenant_id()), @@ -57,6 +51,12 @@ impl PrincipalGet for Server { .await .caused_by(trc::location!())?; + // inbuxa: with directory queries off, only the accounts this one already + // has to do with: its own, its groups, and whoever shared with it. + if !self.can_read_directory(access_token) { + principal_ids &= RoaringBitmap::from_iter(access_token.all_ids()); + } + let ids = if let Some(ids) = ids { ids } else { diff --git a/crates/jmap/src/principal/query.rs b/crates/jmap/src/principal/query.rs index e38de82d..aefc1e91 100644 --- a/crates/jmap/src/principal/query.rs +++ b/crates/jmap/src/principal/query.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::query::QueryResponseBuilder; @@ -14,7 +16,7 @@ use jmap_proto::{ use registry::{ schema::{ enums::AccountType, - prelude::{ObjectType, Permission, Property}, + prelude::{ObjectType, Property}, }, types::EnumImpl, }; @@ -41,9 +43,7 @@ impl PrincipalQuery for Server { mut request: QueryRequest, access_token: &AccessToken, ) -> trc::Result { - if !self.core.groupware.allow_directory_query - && !access_token.has_permission(Permission::JmapPrincipalQuery) - { + if !self.can_read_directory(access_token) { return Err(trc::JmapEvent::Forbidden .into_err() .details("The administrator has disabled directory queries.".to_string())); diff --git a/tests/src/jmap/mod.rs b/tests/src/jmap/mod.rs index 76cc5eec..3b93640c 100644 --- a/tests/src/jmap/mod.rs +++ b/tests/src/jmap/mod.rs @@ -186,6 +186,8 @@ pub async fn jmap_tests() { test.insert_account(admin); + principal::directory_off::test(&test).await; + mail::get::test(&test).await; mail::set::test(&test).await; mail::parse::test(&test).await; diff --git a/tests/src/jmap/principal/directory_off.rs b/tests/src/jmap/principal/directory_off.rs new file mode 100644 index 00000000..b837828f --- /dev/null +++ b/tests/src/jmap/principal/directory_off.rs @@ -0,0 +1,139 @@ +/* + * SPDX-FileCopyrightText: 2026 Coffey Labs LLC + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +use crate::utils::{jmap::JmapUtils, server::TestServer}; +use registry::schema::{prelude::Property, structs::Sharing}; +use serde_json::{Value, json}; + +/// Runs before anything shares between these accounts, so that "already has +/// to do with" means the account itself and its groups. +pub async fn test(test: &TestServer) { + println!("Running directory queries off tests..."); + let admin = test.account("admin"); + let john = test.account("jdoe@example.com"); + let jane = test.account("jane.smith@example.com"); + let john_id = john.id_string(); + let jane_id = jane.id_string(); + + set_directory_queries(test, false).await; + + // A regular user holds every jmapPrincipal* permission and still can't list anyone + assert_eq!( + john.jmap_method_call("Principal/query", json!({ "accountId": john_id })) + .await + .method_response() + .typ(), + "forbidden" + ); + + // Principal/get answers for the account itself and nobody else, asked by id or for everyone + let response = john + .jmap_method_call( + "Principal/get", + json!({ "accountId": john_id, "ids": [john_id, jane_id], "properties": ["id"] }), + ) + .await; + let found = ids(&response.method_response()["list"]); + assert_eq!(found, [john_id], "{response:?}"); + assert_eq!( + response.method_response()["notFound"], + json!([jane_id]), + "{response:?}" + ); + let response = john + .jmap_method_call( + "Principal/get", + json!({ "accountId": john_id, "ids": null, "properties": ["id"] }), + ) + .await; + let found = ids(&response.method_response()["list"]); + assert!(found.contains(&john_id), "{response:?}"); + assert!(!found.contains(&jane_id), "{response:?}"); + + // Free/busy likewise + let availability = |id: &str| { + json!({ + "accountId": john_id, + "id": id, + "utcStart": "2026-01-01T00:00:00Z", + "utcEnd": "2026-01-02T00:00:00Z", + }) + }; + assert_eq!( + john.jmap_method_call("Principal/getAvailability", availability(jane_id)) + .await + .method_response() + .typ(), + "forbidden" + ); + let response = john + .jmap_method_call("Principal/getAvailability", availability(john_id)) + .await; + assert!( + response.method_response()["list"].is_array(), + "{response:?}" + ); + + // Whoever can list accounts anyway isn't held back + let response = admin + .jmap_method_calls(json!([ + ["Principal/query", { "accountId": admin.id_string() }, "q"], + ["Principal/get", { + "accountId": admin.id_string(), + "#ids": { "resultOf": "q", "name": "Principal/query", "path": "/ids" }, + "properties": ["id"] + }, "g"] + ])) + .await; + let found = ids(&response.response_at(1)["list"]); + assert!( + found.contains(&john_id) && found.contains(&jane_id), + "{response:?}" + ); + + // Back on, a regular user can look people up again + set_directory_queries(test, true).await; + let response = john + .jmap_method_calls(json!([ + ["Principal/query", { "accountId": john_id }, "q"], + ["Principal/get", { + "accountId": john_id, + "#ids": { "resultOf": "q", "name": "Principal/query", "path": "/ids" }, + "properties": ["id"] + }, "g"] + ])) + .await; + assert!( + ids(&response.response_at(1)["list"]).contains(&jane_id), + "{response:?}" + ); + + // Asking for free/busy made John's default calendar + john.destroy_all_calendars().await; + test.assert_is_empty().await; +} + +fn ids(list: &Value) -> Vec<&str> { + list.as_array() + .unwrap_or_else(|| panic!("Expected a list: {list:?}")) + .iter() + .map(|p| p["id"].as_str().unwrap()) + .collect() +} + +pub async fn set_directory_queries(test: &TestServer, allow: bool) { + let admin = test.account("admin"); + admin + .registry_update_setting( + Sharing { + allow_directory_queries: allow, + ..Default::default() + }, + &[Property::AllowDirectoryQueries], + ) + .await; + admin.reload_settings().await; +} diff --git a/tests/src/jmap/principal/mod.rs b/tests/src/jmap/principal/mod.rs index 6077706c..a94251ef 100644 --- a/tests/src/jmap/principal/mod.rs +++ b/tests/src/jmap/principal/mod.rs @@ -2,7 +2,10 @@ * SPDX-FileCopyrightText: 2020 Stalwart Labs LLC * * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL + * + * Modified by Coffey Labs in 2026 for INBUXA. */ pub mod availability; +pub mod directory_off; pub mod get; diff --git a/tests/src/webdav/principals.rs b/tests/src/webdav/principals.rs index 78bb1cf1..97c34f1b 100644 --- a/tests/src/webdav/principals.rs +++ b/tests/src/webdav/principals.rs @@ -2,13 +2,16 @@ * 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; +use crate::utils::{server::TestServer, webdav::DummyWebDavClient}; use crate::webdav::prop::ALL_DAV_PROPERTIES; use dav_proto::schema::property::{DavProperty, PrincipalProperty, WebDavProperty}; use groupware::DavResourceName; use hyper::StatusCode; +use registry::schema::{prelude::Property, structs::Sharing}; pub async fn test(test: &TestServer, assisted_discovery: bool) { println!("Running principals tests..."); @@ -483,6 +486,70 @@ pub async fn test(test: &TestServer, assisted_discovery: bool) { ) .as_str()]); + // inbuxa: Test 7: with directory queries off, searching is refused and the + // principal collection lists only the caller's own, while principal-match + // still works, since clients use it to find their own account. + let admin = test.account("admin"); + let set_directory_queries = async |allow| { + admin + .registry_update_setting( + Sharing { + allow_directory_queries: allow, + ..Default::default() + }, + &[Property::AllowDirectoryQueries], + ) + .await; + admin.reload_settings().await; + }; + let principal_href = |name: &str| { + format!( + "{}/{}%40example.com/", + DavResourceName::Principal.base_path(), + name + ) + }; + set_directory_queries(false).await; + client + .request( + "REPORT", + DavResourceName::Principal.collection_path(), + PRINCIPAL_PROPERTY_SEARCH_QUERY.replace("$NAME", "doe"), + ) + .await + .with_status(StatusCode::FORBIDDEN); + client + .request( + "REPORT", + DavResourceName::Principal.collection_path(), + PRINCIPAL_MATCH_QUERY, + ) + .await + .with_status(StatusCode::MULTI_STATUS) + .into_propfind_response(None) + .with_hrefs([ + principal_href("jane").as_str(), + principal_href("support").as_str(), + ]); + // Whoever can list accounts anyway isn't held back + // (not webdav_client(): the administrator has no address) + DummyWebDavClient::new( + admin.id().document_id(), + admin.name(), + admin.secret(), + "admin@example.com", + ) + .request( + "REPORT", + DavResourceName::Principal.collection_path(), + PRINCIPAL_PROPERTY_SEARCH_QUERY.replace("$NAME", "support"), + ) + .await + .with_status(StatusCode::MULTI_STATUS) + .into_propfind_response(None) + .with_hrefs([principal_href("support").as_str()]); + set_directory_queries(true).await; + client.delete_default_containers().await; client .delete_default_containers_by_account("support@example.com") -- 2.54.0