Allow principal resource slots in condition

cedar-policy-core/src/ast/policy.rs

@@ -2365,7 +2365,7 @@ mod test {
                 .exactly_one_underline("?principal")
                 .build()
             );
-            assert_eq!(e.len(), 2);
+            assert_eq!(e.len(), 1);
         });
     }
 

cedar-policy-core/src/est.rs

@@ -280,10 +280,13 @@ impl Policy {
         id: Option<ast::PolicyID>,
     ) -> Result<ast::Template, FromJsonError> {
         let id = id.unwrap_or_else(|| ast::PolicyID::from_string("JSON policy"));
+        let has_principal = self.principal.has_slot();
+        let has_resource = self.resource.has_slot();
+
         let mut conditions_iter = self
             .conditions
             .into_iter()
-            .map(|cond| cond.try_into_ast(&id));
+            .map(|cond| cond.try_into_ast(has_principal, has_resource, &id));
         let conditions = match conditions_iter.next() {
             None => ast::Expr::val(true),
             Some(first) => ast::ExprBuilder::with_data(())
@@ -312,25 +315,47 @@ impl Policy {
 }
 
 impl Clause {
-    fn filter_slots(e: ast::Expr, is_when: bool) -> Result<ast::Expr, FromJsonError> {
-        let first_slot = e.slots().next();
-        if let Some(slot) = first_slot {
-            Err(parse_errors::SlotsInConditionClause {
-                slot,
-                clause_type: if is_when { "when" } else { "unless" },
+    fn filter_slots(
+        e: ast::Expr,
+        has_principal: bool,
+        has_resource: bool,
+        is_when: bool,
+    ) -> Result<ast::Expr, FromJsonError> {
+        for slot in e.slots() {
+            if (slot.id.is_principal() && !has_principal)
+                || (slot.id.is_resource() && !has_resource)
+            {
+                return Err(FromJsonError::SlotsNotInScopeInConditionClause(
+                    parse_errors::SlotsNotInScopeInConditionClause {
+                        slot,
+                        clause_type: if is_when { "when" } else { "unless" },
+                    },
+                ));
             }
-            .into())
-        } else {
-            Ok(e)
         }
+        Ok(e)
     }
+
     /// `id` is the ID of the policy the clause belongs to, used only for reporting errors
-    fn try_into_ast(self, id: &ast::PolicyID) -> Result<ast::Expr, FromJsonError> {
+    /// has_principal/has_resource tells us whether there is a principal/resource slot in the scope
+    /// so we know when a slot is allowed to appear in the condition
+    /// an error is thrown otherwise if there is a slot not in the scope but in the condition
+    fn try_into_ast(
+        self,
+        has_principal: bool,
+        has_resource: bool,
+        id: &ast::PolicyID,
+    ) -> Result<ast::Expr, FromJsonError> {
         match self {
-            Clause::When(expr) => Self::filter_slots(expr.try_into_ast(id)?, true),
-            Clause::Unless(expr) => {
-                Self::filter_slots(ast::Expr::not(expr.try_into_ast(id)?), false)
+            Clause::When(expr) => {
+                Self::filter_slots(expr.try_into_ast(id)?, has_principal, has_resource, true)
             }
+            Clause::Unless(expr) => Self::filter_slots(
+                ast::Expr::not(expr.try_into_ast(id)?),
+                has_principal,
+                has_resource,
+                false,
+            ),
         }
     }
 }
@@ -3575,6 +3600,39 @@ mod test {
                 );
             }
         );
+
+        let template = json!({
+            "effect": "permit",
+            "principal": { "op": "All" },
+            "action": { "op": "All" },
+            "resource": { "op": "All" },
+            "conditions": [
+                {
+                    "kind": "when",
+                    "body": {
+                        "==": {
+                            "left": { "Var": "principal" },
+                            "right": { "Slot": "?principal" }
+                        }
+                    }
+                }
+            ]
+        });
+
+        let est: Policy = serde_json::from_value(template).unwrap();
+        let ast: Result<ast::Policy, _> = est.try_into_ast_policy(None);
+        assert_matches!(
+            ast,
+            Err(e) => {
+                expect_err(
+                    "",
+                    &miette::Report::new(e),
+                    &ExpectedErrorMessageBuilder::error("found template slot ?principal in a `when` clause")
+                    .help("?principal needs to appear in the scope to appear in the condition of the template")
+                    .build(),
+                );
+            }
+        )
     }
 
     #[test]

cedar-policy-core/src/est/err.rs

@@ -53,7 +53,7 @@ pub enum FromJsonError {
     /// EST contained a template slot in policy condition
     #[error(transparent)]
     #[diagnostic(transparent)]
-    SlotsInConditionClause(#[from] parse_errors::SlotsInConditionClause),
+    SlotsNotInScopeInConditionClause(#[from] parse_errors::SlotsNotInScopeInConditionClause),
     /// EST contained the empty JSON object `{}` where a key (operator) was expected
     #[error("missing operator, found empty object")]
     MissingOperator,

cedar-policy-core/src/parser.rs

@@ -813,11 +813,12 @@ mod tests {
                 resource == ?resource
             };
             "#;
-        let slot_in_when_clause =
-            ExpectedErrorMessageBuilder::error("found template slot ?resource in a `when` clause")
-                .help("slots are currently unsupported in `when` clauses")
-                .exactly_one_underline("?resource")
-                .build();
+        let slot_in_when_clause = ExpectedErrorMessageBuilder::error(
+            "found template slot ?resource in a `when` clause",
+        )
+        .help("?resource needs to appear in the scope to appear in the condition of the template")
+        .exactly_one_underline("?resource")
+        .build();
         let unexpected_template = ExpectedErrorMessageBuilder::error(
             "expected a static policy, got a template containing the slot ?resource",
         )
@@ -852,11 +853,12 @@ mod tests {
                 resource == ?principal
             };
             "#;
-        let slot_in_when_clause =
-            ExpectedErrorMessageBuilder::error("found template slot ?principal in a `when` clause")
-                .help("slots are currently unsupported in `when` clauses")
-                .exactly_one_underline("?principal")
-                .build();
+        let slot_in_when_clause = ExpectedErrorMessageBuilder::error(
+            "found template slot ?principal in a `when` clause",
+        )
+        .help("?principal needs to appear in the scope to appear in the condition of the template")
+        .exactly_one_underline("?principal")
+        .build();
         let unexpected_template = ExpectedErrorMessageBuilder::error(
             "expected a static policy, got a template containing the slot ?principal",
         )
@@ -922,7 +924,7 @@ mod tests {
         let slot_in_unless_clause = ExpectedErrorMessageBuilder::error(
             "found template slot ?resource in a `unless` clause",
         )
-        .help("slots are currently unsupported in `unless` clauses")
+        .help("?resource needs to appear in the scope to appear in the condition of the template")
         .exactly_one_underline("?resource")
         .build();
         let unexpected_template = ExpectedErrorMessageBuilder::error(
@@ -962,7 +964,7 @@ mod tests {
         let slot_in_unless_clause = ExpectedErrorMessageBuilder::error(
             "found template slot ?principal in a `unless` clause",
         )
-        .help("slots are currently unsupported in `unless` clauses")
+        .help("?principal needs to appear in the scope to appear in the condition of the template")
         .exactly_one_underline("?principal")
         .build();
         let unexpected_template = ExpectedErrorMessageBuilder::error(
@@ -1029,15 +1031,16 @@ mod tests {
                 resource == ?resource
             };
             "#;
-        let slot_in_when_clause =
-            ExpectedErrorMessageBuilder::error("found template slot ?resource in a `when` clause")
-                .help("slots are currently unsupported in `when` clauses")
-                .exactly_one_underline("?resource")
-                .build();
+        let slot_in_when_clause = ExpectedErrorMessageBuilder::error(
+            "found template slot ?resource in a `when` clause",
+        )
+        .help("?resource needs to appear in the scope to appear in the condition of the template")
+        .exactly_one_underline("?resource")
+        .build();
         let slot_in_unless_clause = ExpectedErrorMessageBuilder::error(
             "found template slot ?resource in a `unless` clause",
         )
-        .help("slots are currently unsupported in `unless` clauses")
+        .help("?resource needs to appear in the scope to appear in the condition of the template")
         .exactly_one_underline("?resource")
         .build();
         let unexpected_template = ExpectedErrorMessageBuilder::error(