reject free variables in expressions

biscuit-auth · Dec 30, 2023 · 4a0cba9 · 4a0cba9
1 parent 06cd200
commit 4a0cba9
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 9 deletions.
diff --git a/src/main/java/com/clevercloud/biscuit/datalog/Rule.java b/src/main/java/com/clevercloud/biscuit/datalog/Rule.java
@@ -75,7 +75,7 @@ public void apply(final Set<Fact> facts, final Set<Fact> new_facts, SymbolTable
                   final Term value = h.get(((Term.Variable) id).value());
                   idit.set(value);
 
-                  // variables that appear in the head should appear in the body and constraints as well
+                  // variables that appear in the head or expressions should appear in the body as well
                   if (value == null) {
                      unbound_variable = true;
                   }

diff --git a/src/main/java/com/clevercloud/biscuit/token/builder/Expression.java b/src/main/java/com/clevercloud/biscuit/token/builder/Expression.java
@@ -3,10 +3,7 @@
 import com.clevercloud.biscuit.datalog.SymbolTable;
 import com.clevercloud.biscuit.datalog.expressions.Op;
 
-import java.util.ArrayDeque;
-import java.util.ArrayList;
-import java.util.Deque;
-import java.util.List;
+import java.util.*;
 
 public abstract class Expression {
     public com.clevercloud.biscuit.datalog.expressions.Expression convert(SymbolTable symbols) {
@@ -119,6 +116,7 @@ public static Expression convert_from(com.clevercloud.biscuit.datalog.expression
     }
 
     public abstract void toOpcodes(SymbolTable symbols, List<com.clevercloud.biscuit.datalog.expressions.Op> ops);
+    public abstract void gatherVariables(Set<String> variables);
 
     public enum Op {
         Negate,
@@ -158,6 +156,12 @@ public void toOpcodes(SymbolTable symbols, List<com.clevercloud.biscuit.datalog.
             ops.add(new com.clevercloud.biscuit.datalog.expressions.Op.Value(this.value.convert(symbols)));
         }
 
+        public  void gatherVariables(Set<String> variables) {
+            if(this.value instanceof Term.Variable) {
+                variables.add(((Term.Variable) this.value).value);
+            }
+        }
+
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;
@@ -206,6 +210,10 @@ public void toOpcodes(SymbolTable symbols, List<com.clevercloud.biscuit.datalog.
             }
         }
 
+        public  void gatherVariables(Set<String> variables) {
+            this.arg1.gatherVariables(variables);
+        }
+
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;
@@ -315,6 +323,11 @@ public void toOpcodes(SymbolTable symbols, List<com.clevercloud.biscuit.datalog.
             }
         }
 
+        public  void gatherVariables(Set<String> variables) {
+            this.arg1.gatherVariables(variables);
+            this.arg2.gatherVariables(variables);
+        }
+
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;

diff --git a/src/main/java/com/clevercloud/biscuit/token/builder/Rule.java b/src/main/java/com/clevercloud/biscuit/token/builder/Rule.java
@@ -100,22 +100,25 @@ public void apply_variables() {
     }
 
     public Either<String, Rule> validate_variables() {
-        Set<String> head_variables = this.head.terms.stream().flatMap(t -> {
+        Set<String> free_variables = this.head.terms.stream().flatMap(t -> {
             if (t instanceof Term.Variable) {
                 return Stream.of(((Term.Variable) t).value);
             } else return Stream.empty();
         }).collect(Collectors.toSet());
+        for(Expression e: this.expressions) {
+            e.gatherVariables(free_variables);
+        }
         for (Predicate p : this.body) {
             for (Term term : p.terms) {
                 if (term instanceof Term.Variable) {
-                    head_variables.remove(((Term.Variable) term).value);
-                    if (head_variables.isEmpty()) {
+                    free_variables.remove(((Term.Variable) term).value);
+                    if (free_variables.isEmpty()) {
                         return Either.right(this);
                     }
                 }
             }
         }
-        return Either.left("rule head contains variables that are not used in predicates of the rule's body: " + head_variables.toString());
+        return Either.left("rule head or expressions contains variables that are not used in predicates of the rule's body: " + free_variables.toString());
     }
 
     public com.clevercloud.biscuit.datalog.Rule convert(SymbolTable symbols) {

diff --git a/src/test/java/com/clevercloud/biscuit/builder/parser/ParserTest.java b/src/test/java/com/clevercloud/biscuit/builder/parser/ParserTest.java
@@ -130,6 +130,19 @@ void testRuleWithExpressionOrdering() {
                 res);
     }
 
+
+    @Test
+    void ruleWithFreeExpressionVariables() {
+        Either<Error, Tuple2<String, Rule>> res =
+                Parser.rule("right($0) <- resource($0), operation(\"read\"), $test");
+        assertEquals(
+                Either.left(
+                        new Error(" resource($0), operation(\"read\"), $test",
+                                "rule head or expressions contains variables that are not used in predicates of the rule's body: [test]")
+                ),
+                res);
+    }
+
     @Test
     void testCheck() {
         Either<Error, Tuple2<String, Check>> res =