From 9feb665bbcb5c2cdb2200a19683f3dd36496c19d Mon Sep 17 00:00:00 2001 From: Clement Delafargue Date: Tue, 31 Oct 2023 10:08:02 +0100 Subject: [PATCH] authorizer: always display authorizer facts and rules `impl Display for Authorizer` only used the datalog world for displaying facts. When calling `.to_string()` on an authorizer before `.authorize()`, authorizer facts and rules were not displayed (authorizer facts and rules are added to the datalog world only in `.authorize()`). After authorization, authorizer facts and rules are both in the world and the block, so deduplication is needed. --- biscuit-auth/src/token/authorizer.rs | 229 +++++++++++++++++++++------ biscuit-auth/tests/capi.rs | 4 +- 2 files changed, 181 insertions(+), 52 deletions(-) diff --git a/biscuit-auth/src/token/authorizer.rs b/biscuit-auth/src/token/authorizer.rs index 6738839a..bb4aa771 100644 --- a/biscuit-auth/src/token/authorizer.rs +++ b/biscuit-auth/src/token/authorizer.rs @@ -1019,103 +1019,153 @@ impl Authorizer { impl std::fmt::Display for Authorizer { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - if !self.world.facts.is_empty() { - write!(f, "// Facts:\n")?; - } - + let mut has_facts = false; let mut all_facts = BTreeMap::new(); for (origin, factset) in &self.world.facts.inner { - let mut facts = Vec::new(); + let mut facts = HashSet::new(); for fact in factset { - facts.push(self.symbols.print_fact(&fact)); + facts.insert(self.symbols.print_fact(fact)); } - facts.sort(); + has_facts = has_facts || !facts.is_empty(); all_facts.insert(origin, facts); } + let builder_facts = self + .authorizer_block_builder + .facts + .iter() + .map(|f| f.to_string()) + .collect::>(); + has_facts = has_facts || !builder_facts.is_empty(); + let mut authorizer_origin = Origin::default(); + authorizer_origin.insert(usize::MAX); + match all_facts.get_mut(&authorizer_origin) { + Some(e) => { + e.extend(builder_facts); + } + None => { + all_facts.insert(&authorizer_origin, builder_facts); + } + } + + if has_facts { + writeln!(f, "// Facts:")?; + } + for (origin, factset) in &all_facts { - write!(f, "// origin: {origin}\n")?; + let mut facts = factset.iter().collect::>(); + facts.sort(); - for fact in factset { - write!(f, "{};\n", fact)?; + if !facts.is_empty() { + writeln!(f, "// origin: {origin}")?; } - } - if !self.world.facts.is_empty() { - write!(f, "\n")?; + for fact in facts { + writeln!(f, "{};", fact)?; + } } - if !self.world.rules.inner.is_empty() { - write!(f, "// Rules:\n")?; + if has_facts { + writeln!(f)?; } - let mut rules_map: BTreeMap> = BTreeMap::new(); + let mut has_rules = false; + let mut rules_map: BTreeMap> = BTreeMap::new(); for ruleset in self.world.rules.inner.values() { + has_rules = has_rules || !ruleset.is_empty(); for (origin, rule) in ruleset { rules_map .entry(*origin) .or_default() - .push(self.symbols.print_rule(&rule)); + .insert(self.symbols.print_rule(rule)); } } + + let builder_rules = self + .authorizer_block_builder + .rules + .iter() + .map(|rule| rule.to_string()) + .collect::>(); + has_rules = has_rules || !builder_rules.is_empty(); + + rules_map + .entry(usize::MAX) + .or_default() + .extend(builder_rules); + + if has_rules { + writeln!(f, "// Rules:")?; + } + for (origin, rule_list) in &rules_map { - if *origin == usize::MAX { - write!(f, "// origin: authorizer\n")?; - } else { - write!(f, "// origin: {origin}\n")?; + if !rule_list.is_empty() { + if *origin == usize::MAX { + writeln!(f, "// origin: authorizer")?; + } else { + writeln!(f, "// origin: {origin}")?; + } } - let mut sorted_rule_list = rule_list.clone(); + let mut sorted_rule_list = rule_list.iter().collect::>(); sorted_rule_list.sort(); for rule in sorted_rule_list { - write!(f, "{};\n", rule)?; + writeln!(f, "{};", rule)?; } } - if !self.world.rules.inner.is_empty() { - write!(f, "\n")?; + if has_rules { + writeln!(f)?; } - if !self.authorizer_block_builder.checks.is_empty() - || self - .blocks - .iter() - .flat_map(|blocks| blocks.iter()) - .any(|block| !block.checks.is_empty()) - { - write!(f, "// Checks:\n")?; + let mut has_checks = false; + let mut checks_map: BTreeMap> = Default::default(); + + if let Some(blocks) = &self.blocks { + for (i, block) in blocks.iter().enumerate() { + let entry = checks_map.entry(i).or_default(); + has_checks = has_checks || !&block.checks.is_empty(); + for check in &block.checks { + entry.push(self.symbols.print_check(check)); + } + } } - if !self.authorizer_block_builder.checks.is_empty() { - write!(f, "// origin: authorizer\n")?; + let authorizer_entry = checks_map.entry(usize::MAX).or_default(); - for check in &self.authorizer_block_builder.checks { - write!(f, "{check};\n")?; - } + has_checks = has_checks || !&self.authorizer_block_builder.checks.is_empty(); + for check in &self.authorizer_block_builder.checks { + authorizer_entry.push(check.to_string()); } - if let Some(blocks) = &self.blocks { - for (i, block) in blocks.iter().enumerate() { - if !block.checks.is_empty() { - write!(f, "// origin: {i}\n")?; + if has_checks { + writeln!(f, "// Checks:")?; + } - for check in &block.checks { - write!(f, "{};\n", self.symbols.print_check(check))?; - } + for (origin, checks) in checks_map { + if !checks.is_empty() { + if origin == usize::MAX { + writeln!(f, "// origin: authorizer")?; + } else { + writeln!(f, "// origin: {origin}")?; } } + + for check in checks { + writeln!(f, "{};", &check)?; + } } - if !self.authorizer_block_builder.checks.is_empty() { - write!(f, "\n")?; + if has_checks { + writeln!(f)?; } if !self.policies.is_empty() { - write!(f, "// Policies:\n")?; + writeln!(f, "// Policies:")?; } for policy in self.policies.iter() { - write!(f, "{policy};\n")?; + writeln!(f, "{policy};")?; } Ok(()) @@ -1302,7 +1352,10 @@ impl AuthorizerExt for Authorizer { mod tests { use std::time::Duration; - use crate::{builder::BlockBuilder, KeyPair}; + use crate::{ + builder::{BiscuitBuilder, BlockBuilder}, + KeyPair, + }; use super::*; @@ -1599,4 +1652,80 @@ mod tests { .unwrap(); assert_eq!(block_facts_query_all_explicit.len(), 0); } + + #[test] + fn authorizer_display_before_and_after_authorization() { + let root = KeyPair::new(); + + let mut token_builder = BiscuitBuilder::new(); + token_builder + .add_code( + r#" + authority_fact(true); + authority_rule($v) <- authority_fact($v); + check if authority_fact(true), authority_rule(true); + "#, + ) + .unwrap(); + let token = token_builder.build(&root).unwrap(); + + let mut authorizer = token.authorizer().unwrap(); + authorizer + .add_code( + r#" + authorizer_fact(true); + authorizer_rule($v) <- authorizer_fact($v); + check if authorizer_fact(true), authorizer_rule(true); + allow if true; + "#, + ) + .unwrap(); + let output_before_authorization = authorizer.to_string(); + + assert!( + output_before_authorization.contains("authorizer_fact(true)"), + "Authorizer.to_string() displays authorizer facts even before running authorize()" + ); + + authorizer.authorize().unwrap(); + + let output_after_authorization = authorizer.to_string(); + assert!( + output_after_authorization.contains("authorizer_rule(true)"), + "Authorizer.to_string() displays generated facts after running authorize()" + ); + + assert_eq!( + r#"// Facts: +// origin: 0 +authority_fact(true); +authority_rule(true); +// origin: authorizer +authorizer_fact(true); +authorizer_rule(true); + +// Rules: +// origin: 0 +authority_rule($v) <- authority_fact($v); +// origin: authorizer +authorizer_rule($v) <- authorizer_fact($v); + +// Checks: +// origin: 0 +check if authority_fact(true), authority_rule(true); +// origin: authorizer +check if authorizer_fact(true), authorizer_rule(true); + +// Policies: +allow if true; +"#, + output_after_authorization + ); + } + + #[test] + fn empty_authorizer_display() { + let authorizer = Authorizer::new(); + assert_eq!("", authorizer.to_string()) + } } diff --git a/biscuit-auth/tests/capi.rs b/biscuit-auth/tests/capi.rs index d9a44264..9d0a1c61 100644 --- a/biscuit-auth/tests/capi.rs +++ b/biscuit-auth/tests/capi.rs @@ -116,10 +116,10 @@ right("file1", "read"); hello("world"); // Checks: -// origin: authorizer -check if right("efgh"); // origin: 1 check if operation("read"); +// origin: authorizer +check if right("efgh"); // Policies: allow if true;