From 89129a54b520902255f4079c2e64eb67cd1575e8 Mon Sep 17 00:00:00 2001 From: Dave Kempe Date: Thu, 28 May 2026 06:43:43 +1000 Subject: [PATCH] fix(connections): hide subfolders the user cannot access Closes #147. ab_list_subfolders checked access to the parent folder but then returned every subfolder unfiltered, so a user who could open a parent saw all of its children regardless of per-child group ACLs - clicking one they weren't entitled to gave "no access". (Top-level folders were already filtered; this was the subfolder gap.) Subfolders are now filtered per child. A folder is shown if the user can access it directly OR can access any descendant of it, so a deeper grant (child with its own allowed_groups and inherit_from_parent=false under a denied folder) is never orphaned out of the tree. Admins still see all. New folder_or_descendant_accessible helper does the recursive (boxed async) OR over resolve_folder_access, short-circuiting on the first accessible folder. No Vault mock harness exists to unit-test the Vault-backed path; verified via build + the logic being a thin recursive wrapper over the already-shipping resolve_folder_access. docs/roles-and-access-control.md gains explicit notes that inaccessible folders are hidden (not shown-then-denied) at every level, the descendant-visibility rule, and how inheritance interacts. --- docs/roles-and-access-control.md | 6 ++++ src/api.rs | 57 +++++++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/docs/roles-and-access-control.md b/docs/roles-and-access-control.md index 61353fd..cbf2d43 100644 --- a/docs/roles-and-access-control.md +++ b/docs/roles-and-access-control.md @@ -130,6 +130,12 @@ Connections folders have group-based access control. Each folder has an `allowed - **Admins** bypass group checks and see all folders - **Operators and powerusers** see only folders where their OIDC groups intersect with the folder's `allowed_groups` - If `allowed_groups` is empty, all authenticated users can see the folder +- Folders the user cannot access are **hidden** from the tree, not shown-then-denied. This applies at every level, including subfolders. +- A folder the user cannot access directly is still shown if they can access one of its descendants, so a deeper grant is never orphaned out of the tree. Access of a child can be granted independently of its parent (see Inheritance below). + +### Inheritance + +A subfolder created with `inherit_from_parent: true` (the default for new subfolders) grants access to anyone who can access its parent. A subfolder with its own non-empty `allowed_groups` and `inherit_from_parent: false` is gated solely by its own list, independent of the parent. ### Example diff --git a/src/api.rs b/src/api.rs index 845a146..6b23274 100644 --- a/src/api.rs +++ b/src/api.rs @@ -1779,6 +1779,42 @@ async fn check_folder_access( } } +/// Returns true if the user can access this folder directly, or can access +/// any descendant of it. Used to decide folder visibility: a folder the user +/// cannot enter is still shown if it is on the path to something they can +/// reach, so deeper grants are never orphaned out of the tree. +/// +/// Recursion is boxed (async). The walk short-circuits on the first +/// accessible folder and is bounded by the actual subtree, which is small in +/// practice; folders the user CAN access return immediately without +/// descending. +fn folder_or_descendant_accessible<'a>( + vault: &'a VaultClient, + scope: &'a str, + path: &'a str, + user_groups: &'a [String], +) -> std::pin::Pin + Send + 'a>> { + Box::pin(async move { + if vault + .resolve_folder_access(scope, path, user_groups) + .await + .unwrap_or(false) + { + return true; + } + // Not directly accessible — keep it only if some descendant is. + if let Ok(subs) = vault.list_subfolders(scope, path).await { + for sub in subs { + let child = sub.path.unwrap_or(sub.name); + if folder_or_descendant_accessible(vault, scope, &child, user_groups).await { + return true; + } + } + } + false + }) +} + /// GET /api/addressbook/folders — List folders visible to the current user. pub async fn ab_list_folders( identity: Option>, @@ -1856,7 +1892,26 @@ pub async fn ab_list_subfolders( } match vault.list_subfolders(&scope, &folder).await { - Ok(subfolders) => Json(json!(subfolders)).into_response(), + Ok(subfolders) => { + // Hide subfolders the user cannot reach. A folder is shown if the + // user can access it directly or can access any descendant of it + // (so deeper grants aren't orphaned out of the tree). Admins see + // everything. Fixes #147: previously every subfolder of an + // accessible parent was returned, so users saw folders they could + // only click into and get "no access". + if id.has_role("admin") { + return Json(json!(subfolders)).into_response(); + } + let user_groups = id.groups(); + let mut visible = Vec::with_capacity(subfolders.len()); + for sf in subfolders { + let path = sf.path.clone().unwrap_or_else(|| sf.name.clone()); + if folder_or_descendant_accessible(&vault, &scope, &path, user_groups).await { + visible.push(sf); + } + } + Json(json!(visible)).into_response() + } Err(crate::vault::VaultError::NotFound) => Json(json!([])).into_response(), Err(e) => ( StatusCode::BAD_GATEWAY,