WIP: fix parameter confusion in macros

macros resolve scope parameters and regular parameters through the `ToAnyParams` trait, losing the ability to discriminate between the two in a static way. This leads to panics instead of compilation errors.

TODO: stop using `ToAnyParams` as it still allows providing a PublicKey where a term is expected

Author: divarvel

biscuit-auth/src/token/builder/check.rs

@@ -111,6 +111,16 @@ impl Check {
         }
     }
 
+    // TODO maybe introduce a conversion trait to support refs, multiple values, non-pk scopes
+    #[cfg(feature = "datalog-macro")]
+    pub fn set_macro_scope_param(
+        &mut self,
+        name: &str,
+        param: PublicKey,
+    ) -> Result<(), error::Token> {
+        self.set_scope_lenient(name, param)
+    }
+
     pub fn validate_parameters(&self) -> Result<(), error::Token> {
         for rule in &self.queries {
             rule.validate_parameters()?;

biscuit-auth/src/token/builder/policy.rs

@@ -104,6 +104,16 @@ impl Policy {
         }
     }
 
+    // TODO maybe introduce a conversion trait to support refs, multiple values, non-pk scopes
+    #[cfg(feature = "datalog-macro")]
+    pub fn set_macro_scope_param(
+        &mut self,
+        name: &str,
+        param: PublicKey,
+    ) -> Result<(), error::Token> {
+        self.set_scope_lenient(name, param)
+    }
+
     pub fn validate_parameters(&self) -> Result<(), error::Token> {
         for query in &self.queries {
             query.validate_parameters()?;

biscuit-auth/src/token/builder/rule.rs

@@ -258,6 +258,16 @@ impl Rule {
         }
     }
 
+    // TODO maybe introduce a conversion trait to support refs, multiple values, non-pk scopes
+    #[cfg(feature = "datalog-macro")]
+    pub fn set_macro_scope_param(
+        &mut self,
+        name: &str,
+        param: PublicKey,
+    ) -> Result<(), error::Token> {
+        self.set_scope_lenient(name, param)
+    }
+
     pub(super) fn apply_parameters(&mut self) {
         if let Some(parameters) = self.parameters.clone() {
             self.head.terms = self

biscuit-auth/tests/macros.rs

@@ -2,7 +2,7 @@
  * Copyright (c) 2019 Geoffroy Couprie <contact@geoffroycouprie.com> and Contributors to the Eclipse Foundation.
  * SPDX-License-Identifier: Apache-2.0
  */
-use biscuit_auth::{builder, datalog::RunLimits, KeyPair};
+use biscuit_auth::{builder, datalog::RunLimits, KeyPair, PublicKey};
 use biscuit_quote::{
     authorizer, authorizer_merge, biscuit, biscuit_merge, block, block_merge, check, fact, policy,
     rule,
@@ -314,3 +314,34 @@ fn ecdsa() {
         r#"rule($0, true) <- fact($0, $1, $2, "my_value", {0}) trusting secp256r1/0245dd01132962da3812911b746b080aed714873c1812e7cefacf13e3880712da0"#,
     );
 }
+
+#[test]
+fn trusting() {
+    // this should only work with a proper `PublicKey` value, and fail when trying to provide a string instead
+    let pubkey: PublicKey =
+        "secp256r1/0245dd01132962da3812911b746b080aed714873c1812e7cefacf13e3880712da0"
+            .parse()
+            .unwrap();
+    let _ = authorizer!(
+        r#"
+    nonce("a"); operation("o"); pathname("p");
+    d($x) <- nonce($x) trusting {pubkey}
+        "#
+    );
+    let _ = rule!(
+        r#"
+    data($nonce, $operation, $pathname)
+      <- nonce($nonce), operation($operation), pathname($pathname)
+
+      trusting {pubkey}
+    "#,
+    );
+
+    /* .build_unauthenticated()
+    .unwrap();
+
+    qry.validate_parameters().unwrap();
+
+    let (nonce, operation, pathname): (String, String, String) =
+        authorizer.query_exactly_one(qry).unwrap(); */
+}

biscuit-quote/src/lib.rs

@@ -204,6 +204,8 @@ struct Builder {
 
     // parameters used in the datalog source
     pub datalog_parameters: HashSet<String>,
+    // scope parameters used in the datalog source
+    pub datalog_scope_parameters: HashSet<String>,
     // parameters provided to the macro
     pub macro_parameters: HashSet<String>,
 
@@ -227,6 +229,7 @@ impl Builder {
             parameters,
 
             datalog_parameters: HashSet::new(),
+            datalog_scope_parameters: HashSet::new(),
             macro_parameters,
 
             facts: Vec::new(),
@@ -286,7 +289,8 @@ impl Builder {
         }
 
         if let Some(parameters) = &rule.scope_parameters {
-            self.datalog_parameters.extend(parameters.keys().cloned());
+            self.datalog_scope_parameters
+                .extend(parameters.keys().cloned());
         }
     }
 
@@ -316,12 +320,17 @@ impl Builder {
     }
 
     fn validate(&self) -> Result<(), error::LanguageError> {
-        if self.macro_parameters.is_subset(&self.datalog_parameters) {
+        let all_parameters = self
+            .datalog_parameters
+            .union(&self.datalog_scope_parameters)
+            .cloned()
+            .collect();
+        if self.macro_parameters.is_subset(&all_parameters) {
             Ok(())
         } else {
             let unused_parameters: Vec<String> = self
                 .macro_parameters
-                .difference(&self.datalog_parameters)
+                .difference(&all_parameters)
                 .cloned()
                 .collect();
             Err(error::LanguageError::Parameters {
@@ -334,6 +343,7 @@ impl Builder {
 
 struct Item {
     parameters: HashSet<String>,
+    scope_parameters: HashSet<String>,
     start: TokenStream,
     middle: TokenStream,
     end: TokenStream,
@@ -348,6 +358,7 @@ impl Item {
                 .flatten()
                 .map(|(name, _)| name.to_owned())
                 .collect(),
+            scope_parameters: HashSet::new(),
             start: quote! {
                 let mut __biscuit_auth_item = #fact;
             },
@@ -360,6 +371,7 @@ impl Item {
     fn rule(rule: &Rule) -> Self {
         Self {
             parameters: Item::rule_params(rule).collect(),
+            scope_parameters: Item::rule_scope_params(rule).collect(),
             start: quote! {
                 let mut __biscuit_auth_item = #rule;
             },
@@ -373,6 +385,11 @@ impl Item {
     fn check(check: &Check) -> Self {
         Self {
             parameters: check.queries.iter().flat_map(Item::rule_params).collect(),
+            scope_parameters: check
+                .queries
+                .iter()
+                .flat_map(Item::rule_scope_params)
+                .collect(),
             start: quote! {
                 let mut __biscuit_auth_item = #check;
             },
@@ -386,6 +403,11 @@ impl Item {
     fn policy(policy: &Policy) -> Self {
         Self {
             parameters: policy.queries.iter().flat_map(Item::rule_params).collect(),
+            scope_parameters: policy
+                .queries
+                .iter()
+                .flat_map(Item::rule_scope_params)
+                .collect(),
             start: quote! {
                 let mut __biscuit_auth_item = #policy;
             },
@@ -400,19 +422,22 @@ impl Item {
         rule.parameters
             .iter()
             .flatten()
-            .map(|(name, _)| name.as_ref())
-            .chain(
-                rule.scope_parameters
-                    .iter()
-                    .flatten()
-                    .map(|(name, _)| name.as_ref()),
-            )
-            .map(str::to_owned)
+            .map(|(name, _)| name.to_string())
+    }
+
+    fn rule_scope_params(rule: &Rule) -> impl Iterator<Item = String> + '_ {
+        rule.scope_parameters
+            .iter()
+            .flatten()
+            .map(|(name, _)| name.to_string())
     }
 
     fn needs_param(&self, name: &str) -> bool {
         self.parameters.contains(name)
     }
+    fn needs_scope_param(&self, name: &str) -> bool {
+        self.scope_parameters.contains(name)
+    }
 
     fn add_param(&mut self, name: &str, clone: bool) {
         let ident = Ident::new(name, Span::call_site());
@@ -427,6 +452,20 @@ impl Item {
             __biscuit_auth_item.set_macro_param(#name, #expr).unwrap();
         });
     }
+
+    fn add_scope_param(&mut self, name: &str, clone: bool) {
+        let ident = Ident::new(name, Span::call_site());
+
+        let expr = if clone {
+            quote! { ::core::clone::Clone::clone(&#ident) }
+        } else {
+            quote! { #ident }
+        };
+
+        self.middle.extend(quote! {
+            __biscuit_auth_item.set_macro_scope_param(#name, #expr).unwrap();
+        });
+    }
 }
 
 impl ToTokens for Item {
@@ -477,6 +516,21 @@ impl ToTokens for Builder {
             }
         }
 
+        for param in &self.datalog_scope_parameters {
+            let mut items = items
+                .iter_mut()
+                .filter(|i| i.needs_scope_param(param))
+                .peekable();
+
+            loop {
+                match (items.next(), items.peek()) {
+                    (Some(cur), Some(_next)) => cur.add_scope_param(param, true),
+                    (Some(cur), None) => cur.add_scope_param(param, false),
+                    (None, _) => break,
+                }
+            }
+        }
+
         let builder_type = &self.builder_type;
         let builder_quote = if let Some(target) = &self.target {
             quote! {
@@ -558,6 +612,12 @@ pub fn rule(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
         }
     }
 
+    for param in &builder.datalog_scope_parameters {
+        if rule_item.needs_scope_param(param) {
+            rule_item.add_scope_param(param, false);
+        }
+    }
+
     (quote! {
         {
             #params_quote
@@ -694,6 +754,12 @@ pub fn check(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
         }
     }
 
+    for param in &builder.datalog_scope_parameters {
+        if check_item.needs_scope_param(param) {
+            check_item.add_scope_param(param, false);
+        }
+    }
+
     (quote! {
         {
             #params_quote
@@ -766,6 +832,12 @@ pub fn policy(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
         }
     }
 
+    for param in &builder.datalog_scope_parameters {
+        if policy_item.needs_scope_param(param) {
+            policy_item.add_scope_param(param, false);
+        }
+    }
+
     (quote! {
         {
             #params_quote