From e3b90e3cfb630ff5fd5408d0a5afe5c08d833eeb Mon Sep 17 00:00:00 2001 From: lip Date: Sun, 14 Sep 2025 22:45:25 +0800 Subject: [PATCH 1/5] feat(rbac): preload and propagate wildcard/pattern domain role links - Preload: when a concrete domain is first accessed/created (get_or_create_role), copy role relationships from matching wildcard/pattern domains so the domain's graph contains inherited links (preload/copy mechanism). - Propagate: when adding a role inheritance (add_link), if a domain-matching function is set, propagate the new Link to all affected/matching domains so subsequent queries find the complete inheritance path in a single domain graph. - Tests: add a unit test that verifies cross-domain inheritance for wildcard/pattern domains (covers both preload and propagation behaviors). --- src/rbac/default_role_manager.rs | 186 ++++++++++++++++++++++++++++++- 1 file changed, 184 insertions(+), 2 deletions(-) diff --git a/src/rbac/default_role_manager.rs b/src/rbac/default_role_manager.rs index e61bee31..9f62f742 100644 --- a/src/rbac/default_role_manager.rs +++ b/src/rbac/default_role_manager.rs @@ -27,7 +27,7 @@ pub struct DefaultRoleManager { domain_matching_fn: Option, } -#[derive(Debug)] +#[derive(Clone, Debug)] enum EdgeVariant { Link, Match, @@ -53,6 +53,9 @@ impl DefaultRoleManager { ) -> NodeIndex { let domain = domain.unwrap_or(DEFAULT_DOMAIN); + // detect whether this is a new domain creation + let is_new_domain = !self.all_domains.contains_key(domain); + let graph = self.all_domains.entry(domain.into()).or_default(); let role_entry = self @@ -97,9 +100,158 @@ impl DefaultRoleManager { } } + // If domain matching function exists and this was a new domain, copy + // role links from matching domains into the newly created domain so + // that BFS will see inherited links in this domain's graph. + if is_new_domain { + if let Some(domain_matching_fn) = self.domain_matching_fn { + let keys: Vec = + self.all_domains.keys().cloned().collect(); + for d in keys { + if d != domain && (domain_matching_fn)(domain, &d) { + self.copy_from_domain(&d, domain); + } + } + } + } + new_role_id } + // propagate a Link addition (name1 -> name2) from `domain` into all + // affected/matching domains. This extracts the inline logic from + // `add_link` so the code is clearer and avoids nested borrows. + fn propagate_link_to_affected_domains( + &mut self, + name1: &str, + name2: &str, + domain: &str, + ) { + let name1_owned = name1.to_string(); + let name2_owned = name2.to_string(); + let affected = self.affected_domain_names(domain); + for d in affected { + // obtain mutable graph and index map for the affected domain + let g = self.all_domains.get_mut(&d).unwrap(); + let idx_map = + self.all_domains_indices.entry(d.clone()).or_default(); + + let idx1 = Self::ensure_node_in_graph(g, idx_map, &name1_owned); + let idx2 = Self::ensure_node_in_graph(g, idx_map, &name2_owned); + + // add Link edge if missing + let has_link = g + .edges_connecting(idx1, idx2) + .any(|e| matches!(*e.weight(), EdgeVariant::Link)); + if !has_link { + g.add_edge(idx1, idx2, EdgeVariant::Link); + } + } + + #[cfg(feature = "cached")] + self.cache.clear(); + } + + // ensure a node with `name` exists in graph `g` and in the provided + // `idx_map`. Returns the NodeIndex for the node. + fn ensure_node_in_graph( + g: &mut StableDiGraph, + idx_map: &mut HashMap>, + name: &str, + ) -> NodeIndex { + if let Some(idx) = idx_map.get(name) { + *idx + } else if let Some(idx) = g.node_indices().find(|&i| g[i] == name) { + idx_map.insert(name.to_string(), idx); + idx + } else { + let ni = g.add_node(name.to_string()); + idx_map.insert(name.to_string(), ni); + ni + } + } + + // return the list of affected domain names (immutable) to avoid nested + // mutable borrows when performing operations across domains + fn affected_domain_names(&self, domain: &str) -> Vec { + let mut res = Vec::new(); + if let Some(domain_matching_fn) = self.domain_matching_fn { + let keys: Vec = self.all_domains.keys().cloned().collect(); + for d in keys { + if d != domain && (domain_matching_fn)(&d, domain) { + res.push(d); + } + } + } + res + } + + // copy all role links and nodes from `src_domain` graph into `dst_domain` graph + fn copy_from_domain(&mut self, src_domain: &str, dst_domain: &str) { + if src_domain == dst_domain { + return; + } + + // ensure both graphs exist + if !self.all_domains.contains_key(src_domain) { + return; + } + + let src_graph = match self.all_domains.get(src_domain) { + Some(g) => g.clone(), + None => return, + }; + + // ensure dst indices map exists + let dst_indices = self + .all_domains_indices + .entry(dst_domain.into()) + .or_default(); + + let dst_graph = self.all_domains.entry(dst_domain.into()).or_default(); + + // copy nodes: ensure names exist in dst and capture mapping + let mut id_map: HashMap, NodeIndex> = + HashMap::new(); + for src_idx in src_graph.node_indices() { + let name = &src_graph[src_idx]; + let dst_idx = if let Some(idx) = dst_indices.get(name) { + *idx + } else { + let new_idx = dst_graph.add_node(name.clone()); + dst_indices.insert(name.clone(), new_idx); + new_idx + }; + id_map.insert(src_idx, dst_idx); + } + + // copy edges: for each edge in src_graph, add equivalent edge in dst if missing + for edge_idx in src_graph.edge_indices() { + if let Some((src_s, src_t)) = src_graph.edge_endpoints(edge_idx) { + if let Some(weight) = src_graph.edge_weight(edge_idx) { + let dst_s = id_map.get(&src_s).unwrap(); + let dst_t = id_map.get(&src_t).unwrap(); + + let need_add = match dst_graph.find_edge(*dst_s, *dst_t) { + Some(idx) => { + // if existing edge is Match but source weight is Link, allow adding Link + !matches!(dst_graph[idx], EdgeVariant::Match) + || !matches!(weight, &EdgeVariant::Match) + } + None => true, + }; + + if need_add { + dst_graph.add_edge(*dst_s, *dst_t, weight.clone()); + } + } + } + } + + #[cfg(feature = "cached")] + self.cache.clear(); + } + fn matched_domains(&self, domain: Option<&str>) -> Vec { let domain = domain.unwrap_or(DEFAULT_DOMAIN); if let Some(domain_matching_fn) = self.domain_matching_fn { @@ -203,8 +355,14 @@ impl RoleManager for DefaultRoleManager { if add_link { graph.add_edge(role1, role2, EdgeVariant::Link); + if let Some(domain_str) = domain { + self.propagate_link_to_affected_domains( + name1, name2, domain_str, + ); + } + #[cfg(feature = "cached")] - self.cache.clear() + self.cache.clear(); } } @@ -787,4 +945,28 @@ mod tests { assert_eq!(vec!["alice"], sort_unstable(rm.get_users("*", None))); } + + #[test] + fn test_cross_domain_role_inheritance_complex() { + use crate::model::key_match; + let mut rm = DefaultRoleManager::new(10); + rm.matching_fn(None, Some(key_match)); + + rm.add_link("editor", "admin", Some("*")); + rm.add_link("viewer", "editor", Some("*")); + + rm.add_link("alice", "editor", Some("domain1")); + rm.add_link("bob", "viewer", Some("domain2")); + + assert!(rm.has_link("alice", "admin", Some("domain1"))); + assert!(rm.has_link("bob", "editor", Some("domain2"))); + assert!(rm.has_link("bob", "admin", Some("domain2"))); + + rm.add_link("charlie", "viewer", Some("domain3")); + assert!(rm.has_link("charlie", "editor", Some("domain3"))); + assert!(rm.has_link("charlie", "admin", Some("domain3"))); + + rm.add_link("super_admin", "admin", Some("domain1")); + assert!(rm.has_link("super_admin", "admin", Some("domain1"))); + } } From a6433f1fb2634afcf531d4949d25ea83215697c6 Mon Sep 17 00:00:00 2001 From: lip Date: Mon, 15 Sep 2025 10:31:04 +0800 Subject: [PATCH 2/5] ci: upgrade actions/cache and actions/checkout to v4 --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3ec49e4c..d3ce6fe1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,7 @@ jobs: rust: [beta, stable] steps: - name: Checkout Repository - uses: actions/checkout@master + uses: actions/checkout@v4 - name: Install Rust toolchain ${{ matrix.rust }} uses: actions-rs/toolchain@v1 with: @@ -36,17 +36,17 @@ jobs: brew install gnu-tar echo PATH="/usr/local/opt/gnu-tar/libexec/gnubin:$PATH" >> $GITHUB_ENV - name: Cache cargo registry - uses: actions/cache@v1 + uses: actions/cache@v4 with: path: ~/.cargo/registry key: ${{ matrix.os }}-${{ matrix.rust }}-cargo-registry-${{ hashFiles('**/Cargo.toml') }}-${{ secrets.CACHE_VERSION }} - name: Cache cargo index - uses: actions/cache@v1 + uses: actions/cache@v4 with: path: ~/.cargo/git key: ${{ matrix.os }}-${{ matrix.rust }}-cargo-index-${{ hashFiles('**/Cargo.toml') }}-${{ secrets.CACHE_VERSION }} - name: Cache cargo build - uses: actions/cache@v1 + uses: actions/cache@v4 with: path: target key: ${{ matrix.os }}-${{ matrix.rust }}-cargo-build-target-${{ hashFiles('**/Cargo.toml') }}-${{ secrets.CACHE_VERSION }} From f4ece55331f7b3240355c0b392740fc7794931d6 Mon Sep 17 00:00:00 2001 From: lip Date: Mon, 15 Sep 2025 11:57:42 +0800 Subject: [PATCH 3/5] fix: resolve Rust compiler warnings in rbac_api.rs and util.rs --- src/rbac_api.rs | 12 ++++++------ src/util.rs | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/rbac_api.rs b/src/rbac_api.rs index 57f224aa..115631ca 100644 --- a/src/rbac_api.rs +++ b/src/rbac_api.rs @@ -907,7 +907,7 @@ mod tests { let adapter = FileAdapter::new("examples/rbac_with_hierarchy_policy.csv"); - let mut e = Enforcer::new(m, adapter).await.unwrap(); + let e = Enforcer::new(m, adapter).await.unwrap(); assert_eq!( vec![vec!["alice", "data1", "read"]], @@ -944,7 +944,7 @@ mod tests { let adapter = FileAdapter::new("examples/rbac_with_hierarchy_policy.csv"); - let mut e = Enforcer::new(m, adapter).await.unwrap(); + let e = Enforcer::new(m, adapter).await.unwrap(); assert_eq!( vec![vec!["alice", "data1", "read"]], @@ -1051,7 +1051,7 @@ mod tests { let adapter = FileAdapter::new( "examples/rbac_with_hierarchy_with_domains_policy.csv", ); - let mut e = Enforcer::new(m, adapter).await.unwrap(); + let e = Enforcer::new(m, adapter).await.unwrap(); assert_eq!( vec![ @@ -1075,7 +1075,7 @@ mod tests { tokio::test )] async fn test_pattern_matching_fn() { - let mut e = Enforcer::new( + let e = Enforcer::new( "examples/rbac_with_pattern_model.conf", "examples/rbac_with_pattern_policy.csv", ) @@ -1120,7 +1120,7 @@ mod tests { tokio::test )] async fn test_pattern_matching_fn_with_domain() { - let mut e = Enforcer::new( + let e = Enforcer::new( "examples/rbac_with_pattern_domain_model.conf", "examples/rbac_with_pattern_domain_policy.csv", ) @@ -1164,7 +1164,7 @@ mod tests { tokio::test )] async fn test_pattern_matching_basic_role() { - let mut e = Enforcer::new( + let e = Enforcer::new( "examples/rbac_basic_role_model.conf", "examples/rbac_basic_role_policy.csv", ) diff --git a/src/util.rs b/src/util.rs index f18defef..757bd795 100644 --- a/src/util.rs +++ b/src/util.rs @@ -36,7 +36,7 @@ pub fn escape_eval(m: &str) -> Cow { ESC_E.replace_all(m, "eval(escape_assertion(${1}))") } -pub fn parse_csv_line>(line: S) -> Option> { +pub fn parse_csv_line<'a, S: AsRef + 'a>(line: S) -> Option> { let line = line.as_ref().trim(); if line.is_empty() || line.starts_with('#') { return None; From 5305b9331f77d4a84b3f6cb22380d16060036dd5 Mon Sep 17 00:00:00 2001 From: lip Date: Wed, 17 Sep 2025 16:58:46 +0800 Subject: [PATCH 4/5] test --- .github/workflows/pull_request.yml | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index c829630d..0a95dde1 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -7,24 +7,31 @@ jobs: name: run benchmark runs-on: ubuntu-latest steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v5 with: ref: ${{github.event.pull_request.head.ref}} repository: ${{github.event.pull_request.head.repo.full_name}} + fetch-depth: 0 - uses: actions-rs/toolchain@v1 with: toolchain: stable override: true profile: minimal - name: Cache cargo - uses: actions/cache@v2.1.4 + uses: actions/cache@v4 with: path: | - target - ~/.cargo/git - ~/.cargo/registry + ~/.cargo/bin/ + ~/.cargo/registry/index/ + ~/.cargo/registry/cache/ + ~/.cargo/git/db/ + target/ key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} - - uses: smrpn/criterion-compare-action@move_to_actions + restore-keys: | + ${{ runner.os }}-cargo- + - name: Install critcmp with force + run: cargo install critcmp --force + - uses: smrpn/criterion-compare-action@master with: cwd: benches token: ${{ secrets.GITHUB_TOKEN }} From d2433a83473500eeb928e5c879654b4578a824b8 Mon Sep 17 00:00:00 2001 From: lip Date: Wed, 17 Sep 2025 20:55:42 +0800 Subject: [PATCH 5/5] delete extra blank line --- src/rbac/default_role_manager.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/rbac/default_role_manager.rs b/src/rbac/default_role_manager.rs index 9f62f742..233d010b 100644 --- a/src/rbac/default_role_manager.rs +++ b/src/rbac/default_role_manager.rs @@ -135,7 +135,6 @@ impl DefaultRoleManager { let g = self.all_domains.get_mut(&d).unwrap(); let idx_map = self.all_domains_indices.entry(d.clone()).or_default(); - let idx1 = Self::ensure_node_in_graph(g, idx_map, &name1_owned); let idx2 = Self::ensure_node_in_graph(g, idx_map, &name2_owned);