From ceb010edca2081c216a7fc477752d2c8be13b9d3 Mon Sep 17 00:00:00 2001 From: Brian Bartman Date: Tue, 25 Aug 2026 17:48:41 +0000 Subject: [PATCH 1/3] tables: capability-mask-aware user grant walk Task 2 of #3376. The user grant walk now takes an explicit `authz::CapabilityMask`, applied at every node emission: each emitted node is `edge_bits & delegatable & mask`. Emission-time application is what makes traversal itself subject to the mask -- a node stripped of Delegate/Assume is terminal, so a mask without Delegate confines the token to direct user grants, and Assume (which makes all of an edge's bits delegatable) cannot re-widen a child beyond the mask. Because each parent is itself masked at emission, the clamp composes identically across any number of hops. `RoleGrant::reachable_nodes` stays unmasked internally: role-to-role authorization answers questions where no user bearer participates. `get_user_capability` masks reachability but passes a reached node's legacy value through un-attenuated -- it's compatibility metadata, never an authorization decision, and may legitimately read broader than the token's effective bits. `reachable_prefixes` omits fully attenuated nodes only when the mask actually hides something (`!mask.is_all()`): a masked token doesn't get to learn the shape of grants it cannot exercise. The omission is deliberately not unconditional, because the walk can already emit empty-bit nodes via delegation intersections and output for a mask that hides nothing must not change. Every existing caller (19 non-test sites: 17 in control-plane-api including the data-plane user token exchanges, 2 in agent discovers) explicitly passes `UNMASKED`, so this commit changes no behavior; the signature change exists to force each future caller to choose a mask. Threading real masks from bearer tokens lands with the Authority extractor (task 3). New tests cover unmasked parity, empty-mask denial, direct-only behavior without Delegate, multi-hop clamping with Delegate, Assume containment, additive multi-path composition within the mask, and fully-attenuated prefix omission with its unmasked contrast. --- crates/agent/src/discovers.rs | 2 + .../control-plane-api/src/live_specs/mod.rs | 1 + .../src/server/authorize_user_collection.rs | 2 + .../src/server/authorize_user_prefix.rs | 3 + .../src/server/authorize_user_task.rs | 2 + crates/control-plane-api/src/server/mod.rs | 2 + .../public/graphql/authorized_prefixes.rs | 33 +- .../src/server/public/graphql/data_planes.rs | 1 + .../src/server/public/graphql/mod.rs | 1 + .../src/server/public/graphql/prefixes.rs | 1 + .../server/public/graphql/storage_mappings.rs | 2 + .../src/server/public/status.rs | 1 + crates/tables/src/behaviors.rs | 377 +++++++++++++++++- 13 files changed, 403 insertions(+), 25 deletions(-) diff --git a/crates/agent/src/discovers.rs b/crates/agent/src/discovers.rs index ca903649de4..a4a3fb05c07 100644 --- a/crates/agent/src/discovers.rs +++ b/crates/agent/src/discovers.rs @@ -180,6 +180,7 @@ impl DiscoverExecutor { row.user_id, &row.capture_name, models::authz::Capability::SpecEdit, + models::authz::CapabilityMask::UNMASKED, ) { // Request an early background refresh: the grant may have been // committed after this Snapshot was taken, and cancelling narrows @@ -209,6 +210,7 @@ impl DiscoverExecutor { row.user_id, &row.data_plane_name, models::Capability::Read, + models::authz::CapabilityMask::UNMASKED, ) .then(|| snapshot.data_plane_by_catalog_name(&row.data_plane_name)) .flatten() diff --git a/crates/control-plane-api/src/live_specs/mod.rs b/crates/control-plane-api/src/live_specs/mod.rs index 518eafa569e..2902cd73cc2 100644 --- a/crates/control-plane-api/src/live_specs/mod.rs +++ b/crates/control-plane-api/src/live_specs/mod.rs @@ -28,6 +28,7 @@ fn partition_by_authorization<'n>( user_id, name, capability, + models::authz::CapabilityMask::UNMASKED, ) }); diff --git a/crates/control-plane-api/src/server/authorize_user_collection.rs b/crates/control-plane-api/src/server/authorize_user_collection.rs index a75a43c100a..c155d465af7 100644 --- a/crates/control-plane-api/src/server/authorize_user_collection.rs +++ b/crates/control-plane-api/src/server/authorize_user_collection.rs @@ -71,6 +71,7 @@ fn evaluate_authorization( *user_id, collection_name, capability, + models::authz::CapabilityMask::UNMASKED, ) { return Err(tonic::Status::permission_denied(format!( "{user_email} is not authorized to {collection_name} for {capability:?}", @@ -85,6 +86,7 @@ fn evaluate_authorization( *user_id, "estuary_support/", models::Capability::Admin, + models::authz::CapabilityMask::UNMASKED, ); if !has_support_access { diff --git a/crates/control-plane-api/src/server/authorize_user_prefix.rs b/crates/control-plane-api/src/server/authorize_user_prefix.rs index 3c484c288c4..50564b7e95d 100644 --- a/crates/control-plane-api/src/server/authorize_user_prefix.rs +++ b/crates/control-plane-api/src/server/authorize_user_prefix.rs @@ -88,6 +88,7 @@ fn evaluate_authorization( *user_id, prefix, capability, + models::authz::CapabilityMask::UNMASKED, ) { return Err(tonic::Status::permission_denied(format!( "{user_email} is not authorized to {prefix} for {capability:?}", @@ -102,6 +103,7 @@ fn evaluate_authorization( *user_id, "estuary_support/", models::Capability::Admin, + models::authz::CapabilityMask::UNMASKED, ); if !has_support_access { @@ -117,6 +119,7 @@ fn evaluate_authorization( *user_id, data_plane_name, models::Capability::Read, + models::authz::CapabilityMask::UNMASKED, ) { return Err(tonic::Status::permission_denied(format!( "{user_email} is not authorized to {data_plane_name}", diff --git a/crates/control-plane-api/src/server/authorize_user_task.rs b/crates/control-plane-api/src/server/authorize_user_task.rs index b78427eed2f..a93b3579070 100644 --- a/crates/control-plane-api/src/server/authorize_user_task.rs +++ b/crates/control-plane-api/src/server/authorize_user_task.rs @@ -95,6 +95,7 @@ fn evaluate_authorization( *user_id, task_name, capability, + models::authz::CapabilityMask::UNMASKED, ) { return Err(tonic::Status::permission_denied(format!( "{user_email} is not authorized to {task_name} for {capability:?}", @@ -109,6 +110,7 @@ fn evaluate_authorization( *user_id, "estuary_support/", models::Capability::Admin, + models::authz::CapabilityMask::UNMASKED, ); if !has_support_access { diff --git a/crates/control-plane-api/src/server/mod.rs b/crates/control-plane-api/src/server/mod.rs index e3eaef0fe71..3d3fd4cd9a2 100644 --- a/crates/control-plane-api/src/server/mod.rs +++ b/crates/control-plane-api/src/server/mod.rs @@ -125,6 +125,7 @@ where *user_id, prefix_or_name.as_ref(), min_capability, + models::authz::CapabilityMask::UNMASKED, ) { return Err(tonic::Status::permission_denied(format!( "{user_email} is not authorized to access prefix or name '{prefix_or_name}' with required capability {min_capability}", @@ -155,6 +156,7 @@ where &snapshot.user_grants, claims.sub, &prefix, + models::authz::CapabilityMask::UNMASKED, ); attach(prefix, capability) }) diff --git a/crates/control-plane-api/src/server/public/graphql/authorized_prefixes.rs b/crates/control-plane-api/src/server/public/graphql/authorized_prefixes.rs index d3186d18170..d4d03d24e88 100644 --- a/crates/control-plane-api/src/server/public/graphql/authorized_prefixes.rs +++ b/crates/control-plane-api/src/server/public/graphql/authorized_prefixes.rs @@ -19,13 +19,18 @@ pub(super) fn authorized_prefixes( // BTreeMap iteration from reachable_prefixes is already prefix-sorted, // so the parent-prune step below can run directly on it. - let prefixes = tables::UserGrant::reachable_prefixes(role_grants, user_grants, user_id) - .into_iter() - .filter(|(prefix, _)| { - prefix_filter.is_none_or(|pf| prefix.starts_with(pf) || pf.starts_with(*prefix)) - }) - .filter(|(_, (bits, _))| bits.is_superset(min_bits)) - .map(|(prefix, _)| prefix.to_string()); + let prefixes = tables::UserGrant::reachable_prefixes( + role_grants, + user_grants, + user_id, + models::authz::CapabilityMask::UNMASKED, + ) + .into_iter() + .filter(|(prefix, _)| { + prefix_filter.is_none_or(|pf| prefix.starts_with(pf) || pf.starts_with(*prefix)) + }) + .filter(|(_, (bits, _))| bits.is_superset(min_bits)) + .map(|(prefix, _)| prefix.to_string()); let mut pruned: Vec = Vec::new(); for p in prefixes { @@ -237,7 +242,12 @@ mod tests { ]); let rg = tables::RoleGrants::new(); - let reachable = tables::UserGrant::reachable_prefixes(&rg, &ug, ALICE); + let reachable = tables::UserGrant::reachable_prefixes( + &rg, + &ug, + ALICE, + models::authz::CapabilityMask::UNMASKED, + ); assert_eq!( reachable["acmeCo/"].0, CapabilityBundle::Editor.capabilities() | CapabilityBundle::TeamAdmin.capabilities(), @@ -274,7 +284,12 @@ mod tests { }, ]); - let reachable = tables::UserGrant::reachable_prefixes(&rg, &ug, ALICE); + let reachable = tables::UserGrant::reachable_prefixes( + &rg, + &ug, + ALICE, + models::authz::CapabilityMask::UNMASKED, + ); assert_eq!( reachable["sharedCo/"].0, CapabilityBundle::Editor.capabilities() | CapabilityBundle::TeamAdmin.capabilities(), diff --git a/crates/control-plane-api/src/server/public/graphql/data_planes.rs b/crates/control-plane-api/src/server/public/graphql/data_planes.rs index e8ad3282fdc..f29aadf0882 100644 --- a/crates/control-plane-api/src/server/public/graphql/data_planes.rs +++ b/crates/control-plane-api/src/server/public/graphql/data_planes.rs @@ -518,6 +518,7 @@ impl DataPlanesQuery { claims.sub, &dp.data_plane_name, models::Capability::Read, + models::authz::CapabilityMask::UNMASKED, ) }) .collect(); diff --git a/crates/control-plane-api/src/server/public/graphql/mod.rs b/crates/control-plane-api/src/server/public/graphql/mod.rs index 9b03bb2db70..f84a5821e1c 100644 --- a/crates/control-plane-api/src/server/public/graphql/mod.rs +++ b/crates/control-plane-api/src/server/public/graphql/mod.rs @@ -70,6 +70,7 @@ fn may_access( env.claims()?.sub, name, capability, + models::authz::CapabilityMask::UNMASKED, )) } diff --git a/crates/control-plane-api/src/server/public/graphql/prefixes.rs b/crates/control-plane-api/src/server/public/graphql/prefixes.rs index 1b2e36dbd61..bd36b72b124 100644 --- a/crates/control-plane-api/src/server/public/graphql/prefixes.rs +++ b/crates/control-plane-api/src/server/public/graphql/prefixes.rs @@ -59,6 +59,7 @@ impl PrefixesQuery { &snapshot.role_grants, &snapshot.user_grants, user_id, + models::authz::CapabilityMask::UNMASKED, ); // Cursor pagination: BTreeMap::range jumps directly to the // first key strictly greater than the previous page's last diff --git a/crates/control-plane-api/src/server/public/graphql/storage_mappings.rs b/crates/control-plane-api/src/server/public/graphql/storage_mappings.rs index 3d57a5522bb..36273b17088 100644 --- a/crates/control-plane-api/src/server/public/graphql/storage_mappings.rs +++ b/crates/control-plane-api/src/server/public/graphql/storage_mappings.rs @@ -537,6 +537,7 @@ fn check_authorization( *user_id, catalog_prefix, models::Capability::Admin, + models::authz::CapabilityMask::UNMASKED, ) { return Err(tonic::Status::permission_denied(format!( "{user_email} is not an authorized as an Admin of catalog prefix '{catalog_prefix}'", @@ -779,6 +780,7 @@ impl StorageMappingsQuery { &snapshot.user_grants, claims.sub, &row.catalog_prefix, + models::authz::CapabilityMask::UNMASKED, ) .ok_or_else(|| { async_graphql::Error::new(format!( diff --git a/crates/control-plane-api/src/server/public/status.rs b/crates/control-plane-api/src/server/public/status.rs index bf41fc71e87..072f283c25e 100644 --- a/crates/control-plane-api/src/server/public/status.rs +++ b/crates/control-plane-api/src/server/public/status.rs @@ -53,6 +53,7 @@ pub(crate) async fn handle_get_status( claims.sub, name, models::Capability::Read, + models::authz::CapabilityMask::UNMASKED, ) }) .collect::>(); diff --git a/crates/tables/src/behaviors.rs b/crates/tables/src/behaviors.rs index a041c92a1ca..498fcfb9aa2 100644 --- a/crates/tables/src/behaviors.rs +++ b/crates/tables/src/behaviors.rs @@ -96,8 +96,17 @@ impl super::RoleGrant { capabilities: EnumSet::from(authz::Capability::Assume), legacy: models::Capability::None, }; + // Role-to-role authorization answers questions where no user bearer + // participates (task authorizations, public role reachability), so a + // capability mask never applies here. pathfinding::directed::bfs::bfs_reach(seed, move |f| { - next_neighbors(f.clone(), role_grants, &[], uuid::Uuid::nil()) + next_neighbors( + f.clone(), + role_grants, + &[], + uuid::Uuid::nil(), + authz::CapabilityMask::UNMASKED, + ) }) .skip(1) } @@ -125,10 +134,19 @@ impl super::RoleGrant { } impl super::UserGrant { + /// Walk the grant graph reachable from `user_id`, attenuated to `mask`. + /// + /// The mask is the ceiling carried by the user's bearer token: every + /// emitted node's capabilities are intersected with it, so a masked + /// walk can only ever see a subset of the unmasked walk's authority. + /// Callers authorizing a request must pass the bearer's actual mask; + /// `UNMASKED` is for unmasked tokens and for paths where no user + /// bearer participates. pub fn reachable_nodes<'a>( role_grants: &'a [super::RoleGrant], user_grants: &'a [super::UserGrant], user_id: uuid::Uuid, + mask: authz::CapabilityMask, ) -> impl Iterator> + 'a { let seed = super::NodeRef { object_role: "", @@ -136,7 +154,7 @@ impl super::UserGrant { legacy: models::Capability::None, }; pathfinding::directed::bfs::bfs_reach(seed, move |f| { - next_neighbors(f.clone(), role_grants, user_grants, user_id) + next_neighbors(f.clone(), role_grants, user_grants, user_id, mask) }) .skip(1) } @@ -154,12 +172,24 @@ impl super::UserGrant { role_grants: &'a [super::RoleGrant], user_grants: &'a [super::UserGrant], user_id: uuid::Uuid, + mask: authz::CapabilityMask, ) -> std::collections::BTreeMap<&'a str, (authz::CapabilitySet, models::Capability)> { let mut out: std::collections::BTreeMap< &'a str, (authz::CapabilitySet, models::Capability), > = Default::default(); - for node in Self::reachable_nodes(role_grants, user_grants, user_id) { + for node in Self::reachable_nodes(role_grants, user_grants, user_id, mask) { + // A fully attenuated node conveys no authority and must not + // surface: a masked token doesn't get to learn the shape (or + // legacy metadata) of grants it cannot exercise. Gated on the + // mask actually hiding something — `is_all()` is a + // leak-prevention test here, never an "unmasked bearer" test — + // because the walk can already emit empty-bit nodes via + // delegation intersections, and a mask which hides nothing must + // keep that legacy output unchanged. + if node.capabilities.is_empty() && !mask.is_all() { + continue; + } let entry = out .entry(node.object_role) .or_insert((authz::CapabilitySet::empty(), models::Capability::None)); @@ -176,8 +206,14 @@ impl super::UserGrant { user_grants: &'a [super::UserGrant], user_id: uuid::Uuid, object_role_or_name: &str, + mask: authz::CapabilityMask, ) -> Option { - Self::reachable_nodes(role_grants, user_grants, user_id) + // The mask gates which nodes are *reachable* (traversal stops where + // the mask strips Delegate/Assume), but the legacy value of a + // reached node passes through un-attenuated: it's compatibility + // metadata, never an authorization decision, and may legitimately + // read broader than the token's effective bits. + Self::reachable_nodes(role_grants, user_grants, user_id, mask) .filter(|n| object_role_or_name.starts_with(n.object_role)) .map(|n| n.legacy) .filter(|c| *c != models::Capability::None) @@ -190,9 +226,10 @@ impl super::UserGrant { subject_user_id: uuid::Uuid, object_role_or_name: &'a str, capability: impl Into, + mask: authz::CapabilityMask, ) -> bool { any_path_satisfies( - Self::reachable_nodes(role_grants, user_grants, subject_user_id), + Self::reachable_nodes(role_grants, user_grants, subject_user_id, mask), object_role_or_name, capability, ) @@ -222,6 +259,7 @@ fn next_neighbors<'a>( role_edges: &'a [super::RoleGrant], user_edges: &'a [super::UserGrant], user_id: uuid::Uuid, + mask: authz::CapabilityMask, ) -> impl Iterator> + 'a { let has_delegate = from.capabilities.contains(authz::Capability::Delegate); let has_assume = from.capabilities.contains(authz::Capability::Assume); @@ -293,7 +331,17 @@ fn next_neighbors<'a>( .flatten() .map(move |g| g.to_node_ref(delegatable)); - p1.chain(p2).chain(p3) + // The mask applies at every emission, not to the walk's result. This is + // what makes traversal itself subject to the mask: a node stripped of + // Delegate/Assume is terminal, so a mask without Delegate confines the + // user to direct grants, and Assume — which passes ALL bits through + // `delegatable` — cannot re-widen a child beyond the mask. Because each + // parent was itself masked at emission, `edge & delegatable & mask` + // composes identically across any number of hops. + p1.chain(p2).chain(p3).map(move |mut node| { + node.capabilities = mask.apply(node.capabilities); + node + }) } impl super::StorageMapping { @@ -306,6 +354,7 @@ impl super::StorageMapping { mod test { use crate::{Import, Imports, RoleGrant, RoleGrants, UserGrant, UserGrants}; use enumset::EnumSet; + use models::authz; use models::authz::{Capability, CapabilityBundle}; #[test] @@ -447,6 +496,7 @@ mod test { uuid::Uuid::nil(), "bobCo/thing", models::Capability::Read, + authz::CapabilityMask::UNMASKED, )); assert!(!UserGrant::is_authorized( &role_grants, @@ -454,6 +504,7 @@ mod test { uuid::Uuid::nil(), "bobCo/thing", models::Capability::Write, + authz::CapabilityMask::UNMASKED, )); assert!(UserGrant::is_authorized( &role_grants, @@ -461,6 +512,7 @@ mod test { uuid::Uuid::nil(), "carolCo/hidden/thing", models::Capability::Read, + authz::CapabilityMask::UNMASKED, )); // User max: admin on aliceCo/widgets/ (propagates to bobCo/burgers/). @@ -470,6 +522,7 @@ mod test { uuid::Uuid::max(), "bobCo/burgers/thing", models::Capability::Admin, + authz::CapabilityMask::UNMASKED, )); } @@ -582,7 +635,8 @@ mod test { &role_grants, &user_grants, user1, - "ops/private/dp/acmeCo/foooo" + "ops/private/dp/acmeCo/foooo", + authz::CapabilityMask::UNMASKED ) ); assert_eq!( @@ -591,7 +645,8 @@ mod test { &role_grants, &user_grants, user2, - "ops/private/dp/acmeCo/foooo" + "ops/private/dp/acmeCo/foooo", + authz::CapabilityMask::UNMASKED ) ); assert_eq!( @@ -600,7 +655,8 @@ mod test { &role_grants, &user_grants, user1, - "different/co/altogether" + "different/co/altogether", + authz::CapabilityMask::UNMASKED ) ); } @@ -643,6 +699,7 @@ mod test { uuid::Uuid::from_bytes([1; 16]), "ops/private/dp/acmeCo/foo", models::Capability::Read, + authz::CapabilityMask::UNMASKED, )); // User 2 has admin on acmeCo/nested/, which also picks up the // acmeCo/ role grants (parent prefix matching). @@ -652,6 +709,7 @@ mod test { uuid::Uuid::from_bytes([2; 16]), "ops/private/dp/acmeCo/foo", models::Capability::Read, + authz::CapabilityMask::UNMASKED, )); } @@ -683,9 +741,14 @@ mod test { user_id: uuid::Uuid, expected: Vec<(&str, EnumSet)>, ) { - let mut nodes: Vec<_> = UserGrant::reachable_nodes(role_grants, user_grants, user_id) - .map(|n| (n.object_role.to_string(), n.capabilities)) - .collect(); + let mut nodes: Vec<_> = UserGrant::reachable_nodes( + role_grants, + user_grants, + user_id, + authz::CapabilityMask::UNMASKED, + ) + .map(|n| (n.object_role.to_string(), n.capabilities)) + .collect(); nodes.sort_by(|a, b| a.0.cmp(&b.0).then(a.1.as_u32().cmp(&b.1.as_u32()))); nodes.dedup(); @@ -705,7 +768,14 @@ mod test { required: EnumSet, ) { assert!( - UserGrant::is_authorized(role_grants, user_grants, user_id, name, required), + UserGrant::is_authorized( + role_grants, + user_grants, + user_id, + name, + required, + authz::CapabilityMask::UNMASKED + ), "expected {user_id} to have {required:?} on {name}", ); } @@ -718,7 +788,14 @@ mod test { required: EnumSet, ) { assert!( - !UserGrant::is_authorized(role_grants, user_grants, user_id, name, required), + !UserGrant::is_authorized( + role_grants, + user_grants, + user_id, + name, + required, + authz::CapabilityMask::UNMASKED + ), "expected {user_id} NOT to have {required:?} on {name}", ); } @@ -1305,8 +1382,13 @@ mod test { }]); let role_grants = RoleGrants::new(); - let nodes: Vec<_> = - UserGrant::reachable_nodes(&role_grants, &user_grants, user_id).collect(); + let nodes: Vec<_> = UserGrant::reachable_nodes( + &role_grants, + &user_grants, + user_id, + authz::CapabilityMask::UNMASKED, + ) + .collect(); assert_eq!(nodes.len(), 1); let node = &nodes[0]; @@ -1700,4 +1782,267 @@ mod test { CapabilityBundle::Viewer.capabilities(), ); } + + fn assert_reachable_masked( + role_grants: &RoleGrants, + user_grants: &UserGrants, + user_id: uuid::Uuid, + mask: authz::CapabilityMask, + expected: Vec<(&str, EnumSet)>, + ) { + let mut nodes: Vec<_> = UserGrant::reachable_nodes(role_grants, user_grants, user_id, mask) + .map(|n| (n.object_role.to_string(), n.capabilities)) + .collect(); + nodes.sort_by(|a, b| a.0.cmp(&b.0).then(a.1.as_u32().cmp(&b.1.as_u32()))); + nodes.dedup(); + + let expected: Vec<(String, EnumSet)> = expected + .into_iter() + .map(|(prefix, caps)| (prefix.to_string(), caps)) + .collect(); + + assert_eq!(nodes, expected); + } + + /// A scenario exercising every traversal mode at once: a direct grant + /// with Delegate, a multi-hop role chain, and an Assume takeover edge. + fn masked_walk_scenario() -> (RoleGrants, UserGrants, uuid::Uuid) { + build_scenario( + vec![ + ( + "acmeCo/", + vec![CapabilityBundle::Editor, CapabilityBundle::Writer], + ), + ("supportCo/", vec![CapabilityBundle::Assume]), + ], + vec![ + ("acmeCo/", "bobCo/shared/", vec![CapabilityBundle::Editor]), + ( + "bobCo/shared/", + "carolCo/upstream/", + vec![CapabilityBundle::Viewer], + ), + ("supportCo/", "daveCo/", vec![CapabilityBundle::Admin]), + ], + ) + } + + #[test] + fn test_masked_walk_unmasked_parity() { + use Capability::*; + + // A mask enabling every capability IS the unmasked mask — one value, + // by construction — and intersection with the full set is the + // identity, so the masked walk with it reproduces the walk every + // other test in this module pins by passing UNMASKED. + assert_eq!( + authz::CapabilityMask::bounded(EnumSet::all()), + authz::CapabilityMask::UNMASKED, + ); + + let (role_grants, user_grants, user_id) = masked_walk_scenario(); + let nodes: Vec<_> = UserGrant::reachable_nodes( + &role_grants, + &user_grants, + user_id, + authz::CapabilityMask::UNMASKED, + ) + .map(|n| (n.object_role.to_string(), n.capabilities, n.legacy)) + .collect(); + assert!( + nodes + .iter() + .any(|(prefix, caps, _)| prefix == "daveCo/" && caps.contains(SpecEdit)) + ); + } + + #[test] + fn test_masked_walk_empty_mask_denies_all() { + // An identity-only token: every node is emitted fully attenuated, + // so no authorization can succeed anywhere and no prefix surfaces. + let (role_grants, user_grants, user_id) = masked_walk_scenario(); + let mask = authz::CapabilityMask::bounded(EnumSet::empty()); + + for object in ["acmeCo/thing", "bobCo/shared/thing", "daveCo/thing"] { + assert!(!UserGrant::is_authorized( + &role_grants, + &user_grants, + user_id, + object, + Capability::CatalogRead, + mask, + )); + } + assert!( + UserGrant::reachable_prefixes(&role_grants, &user_grants, user_id, mask).is_empty() + ); + } + + #[test] + fn test_masked_walk_without_delegate_is_direct_only() { + use Capability::*; + + // The mask strips Delegate from the direct Editor grant, so the + // walk terminates there: bobCo/ and carolCo/ are unreachable even + // though the underlying grants would reach them. The supportCo/ + // Assume edge is likewise stripped (Assume isn't in the mask), so + // daveCo/ is unreachable too. + let (role_grants, user_grants, user_id) = masked_walk_scenario(); + let mask = authz::CapabilityMask::bounded(CatalogRead | JournalRead | SpecEdit); + + assert_reachable_masked( + &role_grants, + &user_grants, + user_id, + mask, + vec![ + ("acmeCo/", CatalogRead | JournalRead | SpecEdit), + ("supportCo/", EnumSet::empty()), + ], + ); + assert!(!UserGrant::is_authorized( + &role_grants, + &user_grants, + user_id, + "bobCo/shared/thing", + Capability::CatalogRead, + mask, + )); + } + + #[test] + fn test_masked_walk_multi_hop_with_delegate() { + use Capability::*; + + // With Delegate in the mask the chain traverses, and every hop is + // clamped: bobCo/ loses SpecEdit's siblings outside the mask, and + // carolCo/ receives Viewer bits clamped to CatalogRead. + let (role_grants, user_grants, user_id) = masked_walk_scenario(); + let mask = authz::CapabilityMask::bounded(CatalogRead | Delegate); + + assert_reachable_masked( + &role_grants, + &user_grants, + user_id, + mask, + vec![ + ("acmeCo/", CatalogRead | Delegate), + ("bobCo/shared/", CatalogRead | Delegate), + ("carolCo/upstream/", EnumSet::from(CatalogRead)), + ("supportCo/", EnumSet::empty()), + ], + ); + assert!(UserGrant::is_authorized( + &role_grants, + &user_grants, + user_id, + "carolCo/upstream/thing", + Capability::CatalogRead, + mask, + )); + } + + #[test] + fn test_masked_walk_assume_containment() { + use Capability::*; + + // Assume makes ALL of an edge's bits delegatable as it passes + // through — which must not restore capabilities outside the mask. + // With Assume in the mask, daveCo/ is reached through the takeover + // edge but its Admin bundle is still clamped to the mask. + let (role_grants, user_grants, user_id) = masked_walk_scenario(); + let mask = authz::CapabilityMask::bounded(CatalogRead | Assume); + + assert_reachable_masked( + &role_grants, + &user_grants, + user_id, + mask, + vec![ + ("acmeCo/", EnumSet::from(CatalogRead)), + ("daveCo/", EnumSet::from(CatalogRead)), + ("supportCo/", EnumSet::from(Assume)), + ], + ); + assert!(!UserGrant::is_authorized( + &role_grants, + &user_grants, + user_id, + "daveCo/thing", + Capability::SpecEdit, + mask, + )); + } + + #[test] + fn test_masked_walk_multi_path_composition() { + use Capability::*; + + // Bits still compose additively across distinct paths, but only + // within the mask: two partial grants jointly authorize a request + // their intersection with the mask covers, and jointly fail one + // requiring a bit the mask removed. + let (role_grants, user_grants, user_id) = build_scenario( + vec![ + ("path1/", vec![CapabilityBundle::Viewer]), + ("path2/", vec![CapabilityBundle::Billing]), + ], + vec![], + ); + // Rebind both direct grants onto the same object prefix. + let user_grants = UserGrants::from_iter(user_grants.into_iter().map(|mut g| { + g.object_role = models::Prefix::new("acmeCo/"); + g + })); + let mask = authz::CapabilityMask::bounded(CatalogRead | ViewBilling | JournalRead); + + assert!(UserGrant::is_authorized( + &role_grants, + &user_grants, + user_id, + "acmeCo/thing", + CatalogRead | ViewBilling, + mask, + )); + // EditBilling is held (via Billing) but masked away. + assert!(!UserGrant::is_authorized( + &role_grants, + &user_grants, + user_id, + "acmeCo/thing", + CatalogRead | EditBilling, + mask, + )); + } + + #[test] + fn test_masked_walk_prefixes_omit_fully_attenuated() { + use Capability::*; + + let (role_grants, user_grants, user_id) = masked_walk_scenario(); + + // Masked: supportCo/ is walked (its Assume seed-edge is emitted, + // fully attenuated) but conveys nothing, so it must not surface — + // a masked token doesn't learn the shape of grants it can't use. + let mask = authz::CapabilityMask::bounded(CatalogRead | Delegate); + let masked = UserGrant::reachable_prefixes(&role_grants, &user_grants, user_id, mask); + assert!(!masked.contains_key("supportCo/")); + assert!(!masked.contains_key("daveCo/")); + assert_eq!( + masked.keys().collect::>(), + vec![&"acmeCo/", &"bobCo/shared/", &"carolCo/upstream/"], + ); + + // Unmasked: today's output is unchanged, including nodes whose + // effective bits are empty (supportCo/'s bare Assume grant emits + // Assume, and daveCo/ arrives with full Admin bits). + let unmasked = UserGrant::reachable_prefixes( + &role_grants, + &user_grants, + user_id, + authz::CapabilityMask::UNMASKED, + ); + assert!(unmasked.contains_key("supportCo/")); + assert!(unmasked.contains_key("daveCo/")); + } } From 58e079900c2219954acbcfb8d27c0d072f72e3cf Mon Sep 17 00:00:00 2001 From: Brian Bartman Date: Tue, 25 Aug 2026 18:49:51 +0000 Subject: [PATCH 2/3] models, tables: correct capability-mask comment inaccuracies Comment-only review pass over the stack-2 changes: - Replace the nonexistent "public role reachability" example on the role-to-role walk with the real caller (prefix-to-data-plane checks). - Fix the multi-hop test comment: SpecEdit itself is masked away, not just its siblings. - Fix the omit-fully-attenuated test comment: the unmasked examples have non-empty bits; what's pinned is that mask-hidden prefixes still surface. - CapabilityMask::apply doc: traversal terminates only when both Delegate and Assume are stripped, per the Assume-containment test. - Drop forward references to artifacts of later stack PRs ("the mint", "upgrade token") that don't exist in the codebase yet. --- crates/models/src/authorizations.rs | 6 +++--- crates/models/src/authz.rs | 10 +++++----- crates/tables/src/behaviors.rs | 15 ++++++++------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/crates/models/src/authorizations.rs b/crates/models/src/authorizations.rs index 8cbfa263e4a..d2792f42490 100644 --- a/crates/models/src/authorizations.rs +++ b/crates/models/src/authorizations.rs @@ -485,9 +485,9 @@ mod test { // A populated mask serializes its names verbatim — including names // this binary doesn't recognize — and they survive a round trip - // intact. Carry-through is load-bearing: an upgrade token's - // unrecognized names must re-mint unchanged rather than being - // dropped by whichever instance happens to re-sign it. + // intact. Carry-through is load-bearing in a mixed-version fleet: + // names minted by a newer instance must pass through an older one + // unchanged rather than being silently dropped. let masked = ControlClaims { capability_mask: Some(vec!["SpecEdit".to_string(), "FutureCapability".to_string()]), ..masked diff --git a/crates/models/src/authz.rs b/crates/models/src/authz.rs index f3852948b59..4a4cde4969e 100644 --- a/crates/models/src/authz.rs +++ b/crates/models/src/authz.rs @@ -103,7 +103,7 @@ impl Capability { /// This type answers *what may be exercised*, never *whether the bearer is /// masked*. A token whose mask happens to enable everything is still a /// deliberately-reduced credential, and surfaces that fail closed for masked -/// bearers (the `/admin` endpoints, the mint) must key on the claim's +/// bearers (such as the `/admin` endpoints) must key on the claim's /// presence — `capability_mask.is_some()` — and never on [`Self::is_all`]. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct CapabilityMask(CapabilitySet); @@ -149,10 +149,10 @@ impl CapabilityMask { /// Attenuate `capabilities` to this mask. /// /// Apply this at each node emission of the user grant walk, never to the - /// walk's result: the mask has to gate `Delegate` itself, so that a mask - /// without it confines the token to direct user grants, and it must not - /// be re-widened by `Assume`, which makes all of an edge's bits - /// delegatable as it passes through. + /// walk's result: the mask has to gate traversal itself, so that a mask + /// without `Delegate` (and `Assume`) confines the token to direct user + /// grants, and it must not be re-widened by `Assume`, which makes all of + /// an edge's bits delegatable as it passes through. pub fn apply(self, capabilities: CapabilitySet) -> CapabilitySet { capabilities & self.0 } diff --git a/crates/tables/src/behaviors.rs b/crates/tables/src/behaviors.rs index 498fcfb9aa2..3fde62365ea 100644 --- a/crates/tables/src/behaviors.rs +++ b/crates/tables/src/behaviors.rs @@ -97,8 +97,8 @@ impl super::RoleGrant { legacy: models::Capability::None, }; // Role-to-role authorization answers questions where no user bearer - // participates (task authorizations, public role reachability), so a - // capability mask never applies here. + // participates (task authorizations, prefix-to-data-plane checks), + // so a capability mask never applies here. pathfinding::directed::bfs::bfs_reach(seed, move |f| { next_neighbors( f.clone(), @@ -1915,8 +1915,9 @@ mod test { use Capability::*; // With Delegate in the mask the chain traverses, and every hop is - // clamped: bobCo/ loses SpecEdit's siblings outside the mask, and - // carolCo/ receives Viewer bits clamped to CatalogRead. + // clamped: bobCo/ loses the Editor bits outside the mask (SpecEdit, + // JournalRead), and carolCo/ receives Viewer bits clamped to + // CatalogRead. let (role_grants, user_grants, user_id) = masked_walk_scenario(); let mask = authz::CapabilityMask::bounded(CatalogRead | Delegate); @@ -2033,9 +2034,9 @@ mod test { vec![&"acmeCo/", &"bobCo/shared/", &"carolCo/upstream/"], ); - // Unmasked: today's output is unchanged, including nodes whose - // effective bits are empty (supportCo/'s bare Assume grant emits - // Assume, and daveCo/ arrives with full Admin bits). + // Unmasked: today's output is unchanged, including the prefixes the + // mask hid above (supportCo/'s bare Assume grant emits Assume, and + // daveCo/ arrives with full Admin bits). let unmasked = UserGrant::reachable_prefixes( &role_grants, &user_grants, From 982a84369a18ff8a4fdc15a5b1902b390842974b Mon Sep 17 00:00:00 2001 From: Brian Bartman Date: Wed, 26 Aug 2026 10:45:41 +0000 Subject: [PATCH 3/3] models, tables: capability-mask renames and masked-walk coverage Rename for intent, per review: - CapabilityMask::UNMASKED -> ALL_CAPABILITIES, pairing with the predicate it feeds. - CapabilityMask::is_all -> has_all_capabilities, so the name itself says it answers "does this mask attenuate nothing?" and can't be misread as an "is this bearer unmasked?" check (that's capability_mask.is_some(), a property of the claim). Call sites in control-plane-api and agent update mechanically. Fill two masked-walk coverage gaps: - test_masked_walk_parent_prefix_pickup: the upward traversal mode (parent-prefix role-grant subjects) is clamped and Delegate-gated like the downward one. - test_masked_walk_get_user_capability: a reached node's legacy value passes through un-attenuated (even under an empty mask, for direct grants) while the mask still gates which nodes are reached at all. --- crates/agent/src/discovers.rs | 4 +- .../control-plane-api/src/live_specs/mod.rs | 2 +- .../src/server/authorize_user_collection.rs | 4 +- .../src/server/authorize_user_prefix.rs | 6 +- .../src/server/authorize_user_task.rs | 4 +- crates/control-plane-api/src/server/mod.rs | 4 +- .../public/graphql/authorized_prefixes.rs | 6 +- .../src/server/public/graphql/data_planes.rs | 2 +- .../src/server/public/graphql/mod.rs | 2 +- .../src/server/public/graphql/prefixes.rs | 2 +- .../server/public/graphql/storage_mappings.rs | 4 +- .../src/server/public/status.rs | 2 +- crates/models/src/authorizations.rs | 2 +- crates/models/src/authz.rs | 42 ++--- crates/tables/src/behaviors.rs | 146 +++++++++++++++--- 15 files changed, 169 insertions(+), 63 deletions(-) diff --git a/crates/agent/src/discovers.rs b/crates/agent/src/discovers.rs index a4a3fb05c07..84c4d7fc05e 100644 --- a/crates/agent/src/discovers.rs +++ b/crates/agent/src/discovers.rs @@ -180,7 +180,7 @@ impl DiscoverExecutor { row.user_id, &row.capture_name, models::authz::Capability::SpecEdit, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ) { // Request an early background refresh: the grant may have been // committed after this Snapshot was taken, and cancelling narrows @@ -210,7 +210,7 @@ impl DiscoverExecutor { row.user_id, &row.data_plane_name, models::Capability::Read, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ) .then(|| snapshot.data_plane_by_catalog_name(&row.data_plane_name)) .flatten() diff --git a/crates/control-plane-api/src/live_specs/mod.rs b/crates/control-plane-api/src/live_specs/mod.rs index 2902cd73cc2..f6101d180bd 100644 --- a/crates/control-plane-api/src/live_specs/mod.rs +++ b/crates/control-plane-api/src/live_specs/mod.rs @@ -28,7 +28,7 @@ fn partition_by_authorization<'n>( user_id, name, capability, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ) }); diff --git a/crates/control-plane-api/src/server/authorize_user_collection.rs b/crates/control-plane-api/src/server/authorize_user_collection.rs index c155d465af7..688b1aad40a 100644 --- a/crates/control-plane-api/src/server/authorize_user_collection.rs +++ b/crates/control-plane-api/src/server/authorize_user_collection.rs @@ -71,7 +71,7 @@ fn evaluate_authorization( *user_id, collection_name, capability, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ) { return Err(tonic::Status::permission_denied(format!( "{user_email} is not authorized to {collection_name} for {capability:?}", @@ -86,7 +86,7 @@ fn evaluate_authorization( *user_id, "estuary_support/", models::Capability::Admin, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ); if !has_support_access { diff --git a/crates/control-plane-api/src/server/authorize_user_prefix.rs b/crates/control-plane-api/src/server/authorize_user_prefix.rs index 50564b7e95d..573bf258194 100644 --- a/crates/control-plane-api/src/server/authorize_user_prefix.rs +++ b/crates/control-plane-api/src/server/authorize_user_prefix.rs @@ -88,7 +88,7 @@ fn evaluate_authorization( *user_id, prefix, capability, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ) { return Err(tonic::Status::permission_denied(format!( "{user_email} is not authorized to {prefix} for {capability:?}", @@ -103,7 +103,7 @@ fn evaluate_authorization( *user_id, "estuary_support/", models::Capability::Admin, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ); if !has_support_access { @@ -119,7 +119,7 @@ fn evaluate_authorization( *user_id, data_plane_name, models::Capability::Read, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ) { return Err(tonic::Status::permission_denied(format!( "{user_email} is not authorized to {data_plane_name}", diff --git a/crates/control-plane-api/src/server/authorize_user_task.rs b/crates/control-plane-api/src/server/authorize_user_task.rs index a93b3579070..02bbf959e16 100644 --- a/crates/control-plane-api/src/server/authorize_user_task.rs +++ b/crates/control-plane-api/src/server/authorize_user_task.rs @@ -95,7 +95,7 @@ fn evaluate_authorization( *user_id, task_name, capability, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ) { return Err(tonic::Status::permission_denied(format!( "{user_email} is not authorized to {task_name} for {capability:?}", @@ -110,7 +110,7 @@ fn evaluate_authorization( *user_id, "estuary_support/", models::Capability::Admin, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ); if !has_support_access { diff --git a/crates/control-plane-api/src/server/mod.rs b/crates/control-plane-api/src/server/mod.rs index 3d3fd4cd9a2..3199b3bdf6f 100644 --- a/crates/control-plane-api/src/server/mod.rs +++ b/crates/control-plane-api/src/server/mod.rs @@ -125,7 +125,7 @@ where *user_id, prefix_or_name.as_ref(), min_capability, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ) { return Err(tonic::Status::permission_denied(format!( "{user_email} is not authorized to access prefix or name '{prefix_or_name}' with required capability {min_capability}", @@ -156,7 +156,7 @@ where &snapshot.user_grants, claims.sub, &prefix, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ); attach(prefix, capability) }) diff --git a/crates/control-plane-api/src/server/public/graphql/authorized_prefixes.rs b/crates/control-plane-api/src/server/public/graphql/authorized_prefixes.rs index d4d03d24e88..b73368a094d 100644 --- a/crates/control-plane-api/src/server/public/graphql/authorized_prefixes.rs +++ b/crates/control-plane-api/src/server/public/graphql/authorized_prefixes.rs @@ -23,7 +23,7 @@ pub(super) fn authorized_prefixes( role_grants, user_grants, user_id, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ) .into_iter() .filter(|(prefix, _)| { @@ -246,7 +246,7 @@ mod tests { &rg, &ug, ALICE, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ); assert_eq!( reachable["acmeCo/"].0, @@ -288,7 +288,7 @@ mod tests { &rg, &ug, ALICE, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ); assert_eq!( reachable["sharedCo/"].0, diff --git a/crates/control-plane-api/src/server/public/graphql/data_planes.rs b/crates/control-plane-api/src/server/public/graphql/data_planes.rs index f29aadf0882..f660d5b20ed 100644 --- a/crates/control-plane-api/src/server/public/graphql/data_planes.rs +++ b/crates/control-plane-api/src/server/public/graphql/data_planes.rs @@ -518,7 +518,7 @@ impl DataPlanesQuery { claims.sub, &dp.data_plane_name, models::Capability::Read, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ) }) .collect(); diff --git a/crates/control-plane-api/src/server/public/graphql/mod.rs b/crates/control-plane-api/src/server/public/graphql/mod.rs index f84a5821e1c..f0cb599cfb4 100644 --- a/crates/control-plane-api/src/server/public/graphql/mod.rs +++ b/crates/control-plane-api/src/server/public/graphql/mod.rs @@ -70,7 +70,7 @@ fn may_access( env.claims()?.sub, name, capability, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, )) } diff --git a/crates/control-plane-api/src/server/public/graphql/prefixes.rs b/crates/control-plane-api/src/server/public/graphql/prefixes.rs index bd36b72b124..7ef48bef664 100644 --- a/crates/control-plane-api/src/server/public/graphql/prefixes.rs +++ b/crates/control-plane-api/src/server/public/graphql/prefixes.rs @@ -59,7 +59,7 @@ impl PrefixesQuery { &snapshot.role_grants, &snapshot.user_grants, user_id, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ); // Cursor pagination: BTreeMap::range jumps directly to the // first key strictly greater than the previous page's last diff --git a/crates/control-plane-api/src/server/public/graphql/storage_mappings.rs b/crates/control-plane-api/src/server/public/graphql/storage_mappings.rs index 36273b17088..cffeebb821f 100644 --- a/crates/control-plane-api/src/server/public/graphql/storage_mappings.rs +++ b/crates/control-plane-api/src/server/public/graphql/storage_mappings.rs @@ -537,7 +537,7 @@ fn check_authorization( *user_id, catalog_prefix, models::Capability::Admin, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ) { return Err(tonic::Status::permission_denied(format!( "{user_email} is not an authorized as an Admin of catalog prefix '{catalog_prefix}'", @@ -780,7 +780,7 @@ impl StorageMappingsQuery { &snapshot.user_grants, claims.sub, &row.catalog_prefix, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ) .ok_or_else(|| { async_graphql::Error::new(format!( diff --git a/crates/control-plane-api/src/server/public/status.rs b/crates/control-plane-api/src/server/public/status.rs index 072f283c25e..f302a3ffbf2 100644 --- a/crates/control-plane-api/src/server/public/status.rs +++ b/crates/control-plane-api/src/server/public/status.rs @@ -53,7 +53,7 @@ pub(crate) async fn handle_get_status( claims.sub, name, models::Capability::Read, - models::authz::CapabilityMask::UNMASKED, + models::authz::CapabilityMask::ALL_CAPABILITIES, ) }) .collect::>(); diff --git a/crates/models/src/authorizations.rs b/crates/models/src/authorizations.rs index d2792f42490..e37e6a7eb09 100644 --- a/crates/models/src/authorizations.rs +++ b/crates/models/src/authorizations.rs @@ -330,7 +330,7 @@ mod test { // variant is added. for (claim, mask) in &outcomes { if claim.is_none() { - assert_eq!(*mask, CapabilityMask::UNMASKED); + assert_eq!(*mask, CapabilityMask::ALL_CAPABILITIES); } } let bounded: Vec<_> = outcomes diff --git a/crates/models/src/authz.rs b/crates/models/src/authz.rs index 4a4cde4969e..91f652d502d 100644 --- a/crates/models/src/authz.rs +++ b/crates/models/src/authz.rs @@ -92,7 +92,7 @@ impl Capability { /// The mask is an enable/disable filter, never a grant: `apply` is pure /// intersection, so naming a capability the user doesn't hold conveys /// nothing, while omitting one they do hold disables it. An unmasked bearer -/// simply carries the full set ([`Self::UNMASKED`]) and intersects as the +/// simply carries the full set ([`Self::ALL_CAPABILITIES`]) and intersects as the /// identity. /// /// This is a newtype over [`CapabilitySet`] rather than a bare set because @@ -104,7 +104,8 @@ impl Capability { /// masked*. A token whose mask happens to enable everything is still a /// deliberately-reduced credential, and surfaces that fail closed for masked /// bearers (such as the `/admin` endpoints) must key on the claim's -/// presence — `capability_mask.is_some()` — and never on [`Self::is_all`]. +/// presence — `capability_mask.is_some()` — and never on +/// [`Self::has_all_capabilities`]. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct CapabilityMask(CapabilitySet); @@ -116,8 +117,8 @@ impl CapabilityMask { /// /// There is deliberately no `Default` and no `From`: /// every caller must name its mask, and constructing an unrestricted - /// one must be a visible, greppable choice. - pub const UNMASKED: Self = Self(CapabilitySet::all()); + /// one must be a visible choice. + pub const ALL_CAPABILITIES: Self = Self(CapabilitySet::all()); /// A mask enabling exactly `set`. An empty set is valid and yields a /// token which authenticates an identity but authorizes nothing. @@ -127,16 +128,16 @@ impl CapabilityMask { /// Build a mask from a token's verified `capability_mask` claim. /// - /// An absent claim is [`Self::UNMASKED`]; a present claim enables the - /// union of the capability bits of its recognized [`CapabilityBundle`] - /// names, and that includes an empty list — "no mask" and "an empty - /// mask" are distinct on the wire and the difference is load-bearing. - /// Unrecognized names contribute nothing, so a claim naming only names - /// we don't know bounds the token to nothing at all; see - /// [`CapabilityBundle::from_name`]. + /// An absent claim is [`Self::ALL_CAPABILITIES`]; a present claim + /// enables the union of the capability bits of its recognized + /// [`CapabilityBundle`] names, and that includes an empty list — "no + /// mask" and "an empty mask" are distinct on the wire and the + /// difference is load-bearing. Unrecognized names contribute nothing, + /// so a claim naming only names we don't know bounds the token to + /// nothing at all; see [`CapabilityBundle::from_name`]. pub fn from_claim(mask: Option<&[String]>) -> Self { let Some(mask) = mask else { - return Self::UNMASKED; + return Self::ALL_CAPABILITIES; }; Self( mask.iter() @@ -164,7 +165,7 @@ impl CapabilityMask { /// which is safe when the mask hides nothing. It must NEVER stand in /// for "is this bearer unmasked?": that is a property of the claim /// (`capability_mask.is_some()`), not of this value. - pub fn is_all(self) -> bool { + pub fn has_all_capabilities(self) -> bool { self.0 == CapabilitySet::all() } } @@ -477,7 +478,10 @@ mod test { #[test] fn test_capability_mask_from_claim() { // An absent claim is an unmasked token: the full set. - assert_eq!(CapabilityMask::from_claim(None), CapabilityMask::UNMASKED); + assert_eq!( + CapabilityMask::from_claim(None), + CapabilityMask::ALL_CAPABILITIES + ); let cases = [ // Single-capability bundle names enable exactly the bit they @@ -518,7 +522,7 @@ mod test { ); assert_ne!( CapabilityMask::from_claim(Some(&[])), - CapabilityMask::UNMASKED, + CapabilityMask::ALL_CAPABILITIES, ); insta::assert_debug_snapshot!(masks, @r" @@ -554,10 +558,10 @@ mod test { // The unmasked mask is the identity, and enabling every capability // is the same thing by construction. - assert_eq!(CapabilityMask::UNMASKED.apply(editor), editor); + assert_eq!(CapabilityMask::ALL_CAPABILITIES.apply(editor), editor); assert_eq!( CapabilityMask::bounded(CapabilitySet::all()), - CapabilityMask::UNMASKED, + CapabilityMask::ALL_CAPABILITIES, ); // A mask intersects: it can only ever disable bits, and bits it @@ -571,8 +575,8 @@ mod test { CapabilityMask::bounded(CapabilitySet::empty()).apply(editor), CapabilitySet::empty(), ); - assert!(!CapabilityMask::bounded(editor).is_all()); - assert!(CapabilityMask::UNMASKED.is_all()); + assert!(!CapabilityMask::bounded(editor).has_all_capabilities()); + assert!(CapabilityMask::ALL_CAPABILITIES.has_all_capabilities()); } // GraphQL's `CapabilityBit` vocabulary must stay a subset of the claim diff --git a/crates/tables/src/behaviors.rs b/crates/tables/src/behaviors.rs index 3fde62365ea..80f13587709 100644 --- a/crates/tables/src/behaviors.rs +++ b/crates/tables/src/behaviors.rs @@ -105,7 +105,7 @@ impl super::RoleGrant { role_grants, &[], uuid::Uuid::nil(), - authz::CapabilityMask::UNMASKED, + authz::CapabilityMask::ALL_CAPABILITIES, ) }) .skip(1) @@ -140,7 +140,7 @@ impl super::UserGrant { /// emitted node's capabilities are intersected with it, so a masked /// walk can only ever see a subset of the unmasked walk's authority. /// Callers authorizing a request must pass the bearer's actual mask; - /// `UNMASKED` is for unmasked tokens and for paths where no user + /// `ALL_CAPABILITIES` is for unmasked tokens and for paths where no user /// bearer participates. pub fn reachable_nodes<'a>( role_grants: &'a [super::RoleGrant], @@ -182,12 +182,12 @@ impl super::UserGrant { // A fully attenuated node conveys no authority and must not // surface: a masked token doesn't get to learn the shape (or // legacy metadata) of grants it cannot exercise. Gated on the - // mask actually hiding something — `is_all()` is a + // mask actually hiding something — `has_all_capabilities()` is a // leak-prevention test here, never an "unmasked bearer" test — // because the walk can already emit empty-bit nodes via // delegation intersections, and a mask which hides nothing must // keep that legacy output unchanged. - if node.capabilities.is_empty() && !mask.is_all() { + if node.capabilities.is_empty() && !mask.has_all_capabilities() { continue; } let entry = out @@ -496,7 +496,7 @@ mod test { uuid::Uuid::nil(), "bobCo/thing", models::Capability::Read, - authz::CapabilityMask::UNMASKED, + authz::CapabilityMask::ALL_CAPABILITIES, )); assert!(!UserGrant::is_authorized( &role_grants, @@ -504,7 +504,7 @@ mod test { uuid::Uuid::nil(), "bobCo/thing", models::Capability::Write, - authz::CapabilityMask::UNMASKED, + authz::CapabilityMask::ALL_CAPABILITIES, )); assert!(UserGrant::is_authorized( &role_grants, @@ -512,7 +512,7 @@ mod test { uuid::Uuid::nil(), "carolCo/hidden/thing", models::Capability::Read, - authz::CapabilityMask::UNMASKED, + authz::CapabilityMask::ALL_CAPABILITIES, )); // User max: admin on aliceCo/widgets/ (propagates to bobCo/burgers/). @@ -522,7 +522,7 @@ mod test { uuid::Uuid::max(), "bobCo/burgers/thing", models::Capability::Admin, - authz::CapabilityMask::UNMASKED, + authz::CapabilityMask::ALL_CAPABILITIES, )); } @@ -636,7 +636,7 @@ mod test { &user_grants, user1, "ops/private/dp/acmeCo/foooo", - authz::CapabilityMask::UNMASKED + authz::CapabilityMask::ALL_CAPABILITIES ) ); assert_eq!( @@ -646,7 +646,7 @@ mod test { &user_grants, user2, "ops/private/dp/acmeCo/foooo", - authz::CapabilityMask::UNMASKED + authz::CapabilityMask::ALL_CAPABILITIES ) ); assert_eq!( @@ -656,7 +656,7 @@ mod test { &user_grants, user1, "different/co/altogether", - authz::CapabilityMask::UNMASKED + authz::CapabilityMask::ALL_CAPABILITIES ) ); } @@ -699,7 +699,7 @@ mod test { uuid::Uuid::from_bytes([1; 16]), "ops/private/dp/acmeCo/foo", models::Capability::Read, - authz::CapabilityMask::UNMASKED, + authz::CapabilityMask::ALL_CAPABILITIES, )); // User 2 has admin on acmeCo/nested/, which also picks up the // acmeCo/ role grants (parent prefix matching). @@ -709,7 +709,7 @@ mod test { uuid::Uuid::from_bytes([2; 16]), "ops/private/dp/acmeCo/foo", models::Capability::Read, - authz::CapabilityMask::UNMASKED, + authz::CapabilityMask::ALL_CAPABILITIES, )); } @@ -745,7 +745,7 @@ mod test { role_grants, user_grants, user_id, - authz::CapabilityMask::UNMASKED, + authz::CapabilityMask::ALL_CAPABILITIES, ) .map(|n| (n.object_role.to_string(), n.capabilities)) .collect(); @@ -774,7 +774,7 @@ mod test { user_id, name, required, - authz::CapabilityMask::UNMASKED + authz::CapabilityMask::ALL_CAPABILITIES ), "expected {user_id} to have {required:?} on {name}", ); @@ -794,7 +794,7 @@ mod test { user_id, name, required, - authz::CapabilityMask::UNMASKED + authz::CapabilityMask::ALL_CAPABILITIES ), "expected {user_id} NOT to have {required:?} on {name}", ); @@ -1386,7 +1386,7 @@ mod test { &role_grants, &user_grants, user_id, - authz::CapabilityMask::UNMASKED, + authz::CapabilityMask::ALL_CAPABILITIES, ) .collect(); @@ -1702,7 +1702,7 @@ mod test { } #[test] - fn test_role_assume_propagates_all_capabilities() { + fn test_role_assume_propagates_has_all_capabilities() { use Capability::*; let role_grants = build_role_scenario(vec![ @@ -1834,10 +1834,10 @@ mod test { // A mask enabling every capability IS the unmasked mask — one value, // by construction — and intersection with the full set is the // identity, so the masked walk with it reproduces the walk every - // other test in this module pins by passing UNMASKED. + // other test in this module pins by passing ALL_CAPABILITIES. assert_eq!( authz::CapabilityMask::bounded(EnumSet::all()), - authz::CapabilityMask::UNMASKED, + authz::CapabilityMask::ALL_CAPABILITIES, ); let (role_grants, user_grants, user_id) = masked_walk_scenario(); @@ -1845,7 +1845,7 @@ mod test { &role_grants, &user_grants, user_id, - authz::CapabilityMask::UNMASKED, + authz::CapabilityMask::ALL_CAPABILITIES, ) .map(|n| (n.object_role.to_string(), n.capabilities, n.legacy)) .collect(); @@ -2041,9 +2041,111 @@ mod test { &role_grants, &user_grants, user_id, - authz::CapabilityMask::UNMASKED, + authz::CapabilityMask::ALL_CAPABILITIES, ); assert!(unmasked.contains_key("supportCo/")); assert!(unmasked.contains_key("daveCo/")); } + + #[test] + fn test_masked_walk_parent_prefix_pickup() { + use Capability::*; + + // The upward traversal mode: a grant on acmeCo/nested/ picks up + // role grants whose subject is the parent prefix acmeCo/. That + // edge stream is clamped and Delegate-gated exactly like the + // downward one. + let (role_grants, user_grants, user_id) = build_scenario( + vec![("acmeCo/nested/", vec![CapabilityBundle::Editor])], + vec![("acmeCo/", "sharedCo/", vec![CapabilityBundle::Viewer])], + ); + + // With Delegate in the mask the parent-subject edge traverses, + // and sharedCo/'s Viewer bits are clamped to CatalogRead. + let mask = authz::CapabilityMask::bounded(CatalogRead | Delegate); + assert_reachable_masked( + &role_grants, + &user_grants, + user_id, + mask, + vec![ + ("acmeCo/nested/", CatalogRead | Delegate), + ("sharedCo/", EnumSet::from(CatalogRead)), + ], + ); + + // Without Delegate the direct grant is terminal, so the + // parent-prefix pickup never happens. + let mask = authz::CapabilityMask::bounded(CatalogRead | SpecEdit); + assert_reachable_masked( + &role_grants, + &user_grants, + user_id, + mask, + vec![("acmeCo/nested/", CatalogRead | SpecEdit)], + ); + assert!(!UserGrant::is_authorized( + &role_grants, + &user_grants, + user_id, + "sharedCo/thing", + Capability::CatalogRead, + mask, + )); + } + + #[test] + fn test_masked_walk_get_user_capability() { + use Capability::*; + + // Legacy-capability grants, so nodes carry a legacy value the + // mask must NOT attenuate: it's compatibility metadata, not an + // authorization decision. + let user_grants = UserGrants::from_iter([UserGrant { + user_id: uuid::Uuid::from_bytes([1; 16]), + object_role: models::Prefix::new("acmeCo/"), + capability: models::Capability::Admin, + bundles: vec![], + }]); + let role_grants = RoleGrants::from_iter([RoleGrant { + subject_role: models::Prefix::new("acmeCo/"), + object_role: models::Prefix::new("sharedCo/"), + capability: models::Capability::Read, + bundles: vec![], + }]); + let user_id = uuid::Uuid::from_bytes([1; 16]); + + // A reached node's legacy value passes through un-attenuated: + // even an identity-only token reports the legacy metadata of its + // direct grants, while authorization under the same mask denies. + let mask = authz::CapabilityMask::bounded(EnumSet::empty()); + assert_eq!( + UserGrant::get_user_capability(&role_grants, &user_grants, user_id, "acmeCo/", mask), + Some(models::Capability::Admin), + ); + assert!(!UserGrant::is_authorized( + &role_grants, + &user_grants, + user_id, + "acmeCo/thing", + Capability::CatalogRead, + mask, + )); + + // The mask still gates reachability: without Delegate the walk + // terminates at the direct grant, so sharedCo/ has no legacy + // value to report... + assert_eq!( + UserGrant::get_user_capability(&role_grants, &user_grants, user_id, "sharedCo/", mask), + None, + ); + // ...and with Delegate it's reached, reporting its legacy value + // even though the mask attenuates its effective bits to nothing + // beyond CatalogRead. + let mask = authz::CapabilityMask::bounded(CatalogRead | Delegate); + assert_eq!( + UserGrant::get_user_capability(&role_grants, &user_grants, user_id, "sharedCo/", mask), + Some(models::Capability::Read), + ); + } }