RFC 62 implementation

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

@@ -1858,6 +1858,14 @@ impl<T> Expr<T> {
             }
         }
     }
+
+    pub(crate) fn is_true(&self) -> bool {
+        matches!(self.expr_kind(), ExprKind::Lit(Literal::Bool(true)))
+    }
+
+    pub(crate) fn is_false(&self) -> bool {
+        matches!(self.expr_kind(), ExprKind::Lit(Literal::Bool(false)))
+    }
 }
 
 /// AST variables

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

@@ -523,7 +523,7 @@ impl Name {
         self.0.basename().clone().try_into().unwrap()
     }
 
-    /// Test if a [`Name`] is an [`Id`]
+    /// Test if a [`Name`] is an [`UnreservedId`]
     pub fn is_unqualified(&self) -> bool {
         self.0.is_unqualified()
     }

cedar-policy-core/src/evaluator.rs

@@ -5524,7 +5524,7 @@ pub mod test {
     }
 
     #[test]
-    fn parital_if_noerrors() {
+    fn partial_if_noerrors() {
         let guard = Expr::get_attr(Expr::unknown(Unknown::new_untyped("a")), "field".into());
         let cons = Expr::val(1);
         let alt = Expr::val(2);
@@ -6140,4 +6140,72 @@ pub mod test {
         );
         assert_matches!(eval.partial_eval_expr(&e), Err(_));
     }
+
+    #[test]
+    fn interpret_extended_has() {
+        let es = Entities::new();
+        let eval = Evaluator::new(empty_request(), &es, Extensions::none());
+        assert_matches!(eval.interpret_inline_policy(&parse_expr(r#"
+    {a: {b: {c: 1}}} has a.b.c
+        "#).unwrap()), Ok(v) => {
+            assert_eq!(v, Value::from(true));
+        });
+        assert_matches!(eval.interpret_inline_policy(&parse_expr(r#"
+    {a: {b: {c: 1}}} has a.b
+        "#).unwrap()), Ok(v) => {
+            assert_eq!(v, Value::from(true));
+        });
+        assert_matches!(eval.interpret_inline_policy(&parse_expr(r#"
+    {a: {b: {c: 1}}} has a
+        "#).unwrap()), Ok(v) => {
+            assert_eq!(v, Value::from(true));
+        });
+        assert_matches!(eval.interpret_inline_policy(&parse_expr(r#"
+    {a: {b: {c: 1}}} has b.c
+        "#).unwrap()), Ok(v) => {
+            assert_eq!(v, Value::from(false));
+        });
+        assert_matches!(eval.interpret_inline_policy(&parse_expr(r#"
+    {a: {b: {c: 1}}} has c
+        "#).unwrap()), Ok(v) => {
+            assert_eq!(v, Value::from(false));
+        });
+        assert_matches!(eval.interpret_inline_policy(&parse_expr(r#"
+    {a: {b: {c: 1}}} has d
+        "#).unwrap()), Ok(v) => {
+            assert_eq!(v, Value::from(false));
+        });
+        assert_matches!(eval.interpret_inline_policy(&parse_expr(r#"
+    {a: {b: {c: 1}}} has "🚫"
+        "#).unwrap()), Ok(v) => {
+            assert_eq!(v, Value::from(false));
+        });
+
+        assert_matches!(eval.interpret_inline_policy(&parse_expr(r#"
+    {a: {b: {c: 1}}} has a.b.c && {a: {b: {c: 1}}}.a.b.c == 1
+        "#).unwrap()), Ok(v) => {
+            assert_eq!(v, Value::from(true));
+        });
+        assert_matches!(eval.interpret_inline_policy(&parse_expr(r#"
+    {a: {b: {c: 1}}} has a.b && {a: {b: {c: 1}}}.a.b == {c: 1}
+        "#).unwrap()), Ok(v) => {
+            assert_eq!(v, Value::from(true));
+        });
+        assert_matches!(eval.interpret_inline_policy(&parse_expr(r#"
+    {a: {b: {c: 1}}} has a && {a: {b: {c: 1}}}.a == {b: {c: 1}}
+        "#).unwrap()), Ok(v) => {
+            assert_eq!(v, Value::from(true));
+        });
+        assert_matches!(eval.interpret_inline_policy(&parse_expr(r#"
+    {a: {b: {d: 1}}} has a.b.c && {a: {b: {d: 1}}}.a.b.c == 1
+        "#).unwrap()), Ok(v) => {
+            assert_eq!(v, Value::from(false));
+        });
+
+        assert_matches!(eval.interpret_inline_policy(&parse_expr(r#"
+        {a: {b: {c: 1}}} has a.b && {a: {b: {c: 1}}}.a.b.d == 1
+            "#).unwrap()), Err(EvaluationError::RecordAttrDoesNotExist(err)) => {
+            assert_eq!(err.attr, "d");
+        });
+    }
 }

cedar-policy-core/src/parser/cst_to_ast.rs

@@ -46,6 +46,7 @@ use crate::ast::{
 use crate::est::extract_single_argument;
 use crate::fuzzy_match::fuzzy_search_limited;
 use itertools::Either;
+use nonempty::nonempty;
 use nonempty::NonEmpty;
 use smol_str::{SmolStr, ToSmolStr};
 use std::cmp::Ordering;
@@ -947,10 +948,13 @@ impl Node<Option<cst::Relation>> {
             }
             cst::Relation::Has { target, field } => {
                 let maybe_target = target.to_expr();
-                let maybe_field = field.to_expr_or_special()?.into_valid_attr();
+                let maybe_field = Ok(match field.to_has_rhs()? {
+                    Either::Left(s) => nonempty![s],
+                    Either::Right(ids) => ids.map(|id| id.to_smolstr()),
+                });
                 let (target, field) = flatten_tuple_2(maybe_target, maybe_field)?;
                 Ok(ExprOrSpecial::Expr {
-                    expr: construct_expr_has(target, field, self.loc.clone()),
+                    expr: construct_exprs_extended_has(target, field, self.loc.clone()),
                     loc: self.loc.clone(),
                 })
             }
@@ -998,6 +1002,88 @@ impl Node<Option<cst::Add>> {
     fn to_expr(&self) -> Result<ast::Expr> {
         self.to_expr_or_special()?.into_expr()
     }
+
+    // Peel the grammar onion until we see valid RHS
+    pub(crate) fn to_has_rhs(&self) -> Result<Either<SmolStr, NonEmpty<UnreservedId>>> {
+        let inner @ cst::Add { initial, extended } = self.try_as_inner()?;
+        let err = |loc| {
+            ToASTError::new(ToASTErrorKind::InvalidHasRHS(inner.to_string().into()), loc).into()
+        };
+        let construct_attrs = |first: UnreservedId, rest: &[Node<Option<cst::MemAccess>>]| {
+            let mut acc = nonempty![first];
+            rest.iter().try_for_each(|ma_node| {
+                let ma = ma_node.try_as_inner()?;
+                match ma {
+                    cst::MemAccess::Field(id) => {
+                        acc.push(id.to_unreserved_ident()?);
+                        Ok(())
+                    }
+                    _ => Err(err(ma_node.loc.clone())),
+                }
+            })?;
+            Ok::<nonempty::NonEmpty<UnreservedId>, ParseErrors>(acc)
+        };
+        if !extended.is_empty() {
+            return Err(err(self.loc.clone()));
+        }
+        let cst::Mult { initial, extended } = initial.try_as_inner()?;
+        if !extended.is_empty() {
+            return Err(err(self.loc.clone()));
+        }
+        if let cst::Unary {
+            op: None,
+            item: item_node,
+        } = initial.try_as_inner()?
+        {
+            let cst::Member { item, access } = item_node.try_as_inner()?;
+            let item = item.to_expr_or_special()?;
+            match (item, access.as_slice()) {
+                (ExprOrSpecial::StrLit { lit, loc }, []) => Ok(Either::Left(
+                    to_unescaped_string(lit).map_err(|escape_errs| {
+                        ParseErrors::new_from_nonempty(escape_errs.map(|e| {
+                            ToASTError::new(ToASTErrorKind::Unescape(e), loc.clone()).into()
+                        }))
+                    })?,
+                )),
+                (ExprOrSpecial::Var { var, .. }, rest) => {
+                    // PANIC SAFETY: any variable should be a valid identifier
+                    #[allow(clippy::unwrap_used)]
+                    let first = construct_string_from_var(var).parse().unwrap();
+                    Ok(Either::Right(construct_attrs(first, rest)?))
+                }
+                (ExprOrSpecial::Name { name, loc }, rest) => {
+                    if name.is_unqualified() {
+                        let first = name.basename();
+
+                        Ok(Either::Right(construct_attrs(first, rest)?))
+                    } else {
+                        Err(ToASTError::new(
+                            ToASTErrorKind::PathAsAttribute(inner.to_string()),
+                            loc,
+                        )
+                        .into())
+                    }
+                }
+                // Attempt to return a precise error message for RHS like `true.<...>`
+                (ExprOrSpecial::Expr { loc, expr }, _) if expr.is_true() => Err(ToASTError::new(
+                    ToASTErrorKind::ReservedIdentifier(cst::Ident::True),
+                    loc,
+                )
+                .into()),
+                // Attempt to return a precise error message for RHS like `false.<...>`
+                (ExprOrSpecial::Expr { loc, expr }, _) if expr.is_false() => Err(ToASTError::new(
+                    ToASTErrorKind::ReservedIdentifier(cst::Ident::False),
+                    loc,
+                )
+                .into()),
+                (ExprOrSpecial::Expr { loc, .. }, _) => Err(err(loc)),
+                _ => Err(err(self.loc.clone())),
+            }
+        } else {
+            Err(err(self.loc.clone()))
+        }
+    }
+
     pub(crate) fn to_expr_or_special(&self) -> Result<ExprOrSpecial<'_>> {
         let add = self.try_as_inner()?;
 
@@ -1776,9 +1862,31 @@ fn construct_expr_mul(
     }
     expr
 }
-fn construct_expr_has(t: ast::Expr, s: SmolStr, loc: Loc) -> ast::Expr {
+
+fn construct_expr_has_attr(t: ast::Expr, s: SmolStr, loc: Loc) -> ast::Expr {
     ast::ExprBuilder::new().with_source_loc(loc).has_attr(t, s)
 }
+fn construct_expr_get_attr(t: ast::Expr, s: SmolStr, loc: Loc) -> ast::Expr {
+    ast::ExprBuilder::new().with_source_loc(loc).get_attr(t, s)
+}
+fn construct_exprs_extended_has(t: ast::Expr, attrs: NonEmpty<SmolStr>, loc: Loc) -> ast::Expr {
+    let (first, rest) = attrs.split_first();
+    let has_expr = construct_expr_has_attr(t.clone(), first.to_owned(), loc.clone());
+    let get_expr = construct_expr_get_attr(t, first.to_owned(), loc.clone());
+    rest.iter()
+        .fold((has_expr, get_expr), |(has_expr, get_expr), attr| {
+            (
+                construct_expr_and(
+                    has_expr,
+                    construct_expr_has_attr(get_expr.clone(), attr.to_owned(), loc.clone()),
+                    std::iter::empty(),
+                    &loc,
+                ),
+                construct_expr_get_attr(get_expr, attr.to_owned(), loc.clone()),
+            )
+        })
+        .0
+}
 fn construct_expr_attr(e: ast::Expr, s: SmolStr, loc: Loc) -> ast::Expr {
     ast::ExprBuilder::new().with_source_loc(loc).get_attr(e, s)
 }
@@ -2693,8 +2801,8 @@ mod tests {
         expect_some_error_matches(
             src,
             &errs,
-            &ExpectedErrorMessageBuilder::error("invalid attribute name: 1")
-                .help("attribute names can either be identifiers or string literals")
+            &ExpectedErrorMessageBuilder::error("invalid RHS of a `has` operation: 1")
+                .help("valid RHS of a `has` operation is either a sequence of identifiers separated by `.` or a string literal")
                 .exactly_one_underline("1")
                 .build(),
         );
@@ -2908,8 +3016,8 @@ mod tests {
         expect_some_error_matches(
             src,
             &errs,
-            &ExpectedErrorMessageBuilder::error("invalid attribute name: 1")
-                .help("attribute names can either be identifiers or string literals")
+            &ExpectedErrorMessageBuilder::error("invalid RHS of a `has` operation: 1")
+                .help("valid RHS of a `has` operation is either a sequence of identifiers separated by `.` or a string literal")
                 .exactly_one_underline("1")
                 .build(),
         );
@@ -4719,4 +4827,81 @@ mod tests {
             );
         });
     }
+
+    #[test]
+    fn extended_has() {
+        assert_matches!(
+            parse_policy(
+                None,
+                r#"
+        permit(
+  principal is User,
+  action == Action::"preview",
+  resource == Movie::"Blockbuster"
+) when {
+  principal has contactInfo.address.zip &&
+  principal.contactInfo.address.zip == "90210"
+};
+        "#
+            ),
+            Ok(_)
+        );
+
+        assert_matches!(parse_expr(r#"context has a.b"#), Ok(e) => {
+            assert!(e.eq_shape(&parse_expr(r#"(context has a) && (context.a has b)"#).unwrap()));
+        });
+
+        assert_matches!(parse_expr(r#"context has a.b.c"#), Ok(e) => {
+            assert!(e.eq_shape(&parse_expr(r#"((context has a) && (context.a has b)) && (context.a.b has c)"#).unwrap()));
+        });
+
+        let policy = r#"permit(principal, action, resource) when {
+            principal has a.if
+          };"#;
+        assert_matches!(
+            parse_policy(None, policy),
+            Err(e) => {
+                expect_n_errors(policy, &e, 1);
+                expect_some_error_matches(policy, &e, &ExpectedErrorMessageBuilder::error(
+                    "this identifier is reserved and cannot be used: if",
+                ).exactly_one_underline(r#"if"#).build());
+            }
+        );
+        let policy = r#"permit(principal, action, resource) when {
+            principal has if.a
+          };"#;
+        assert_matches!(
+            parse_policy(None, policy),
+            Err(e) => {
+                expect_n_errors(policy, &e, 1);
+                expect_some_error_matches(policy, &e, &ExpectedErrorMessageBuilder::error(
+                    "this identifier is reserved and cannot be used: if",
+                ).exactly_one_underline(r#"if"#).build());
+            }
+        );
+        let policy = r#"permit(principal, action, resource) when {
+            principal has true.if
+          };"#;
+        assert_matches!(
+            parse_policy(None, policy),
+            Err(e) => {
+                expect_n_errors(policy, &e, 1);
+                expect_some_error_matches(policy, &e, &ExpectedErrorMessageBuilder::error(
+                    "this identifier is reserved and cannot be used: true",
+                ).exactly_one_underline(r#"true"#).build());
+            }
+        );
+        let policy = r#"permit(principal, action, resource) when {
+            principal has a.__cedar
+          };"#;
+        assert_matches!(
+            parse_policy(None, policy),
+            Err(e) => {
+                expect_n_errors(policy, &e, 1);
+                expect_some_error_matches(policy, &e, &ExpectedErrorMessageBuilder::error(
+                    "The name `__cedar` contains `__cedar`, which is reserved",
+                ).exactly_one_underline(r#"__cedar"#).build());
+            }
+        );
+    }
 }

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

@@ -237,6 +237,10 @@ pub enum ToASTErrorKind {
     #[error("invalid attribute name: {0}")]
     #[diagnostic(help("attribute names can either be identifiers or string literals"))]
     InvalidAttribute(SmolStr),
+    /// Returned when the RHS of a `has` operation is invalid
+    #[error("invalid RHS of a `has` operation: {0}")]
+    #[diagnostic(help("valid RHS of a `has` operation is either a sequence of identifiers separated by `.` or a string literal"))]
+    InvalidHasRHS(SmolStr),
     /// Returned when attempting to use an attribute with a namespace
     #[error("`{0}` cannot be used as an attribute as it contains a namespace")]
     PathAsAttribute(String),

cedar-policy-core/src/parser/grammar.lalrpop

@@ -172,6 +172,12 @@ AnyIdent: Node<Option<cst::Ident>> = {
 }
 pub Ident: Node<Option<cst::Ident>> = AnyIdent;
 
+#[inline]
+IfIdent: Node<Option<cst::Ident>> = {
+    <l:@L> IF <r:@R>
+        => Node::with_source_loc(Some(cst::Ident::If), Loc::new(l..r, Arc::clone(src))),
+}
+
 // Cond := ('when' | 'unless') '{' Expr '}'
 Cond: Node<Option<cst::Cond>> = {
     <l:@L> <i:AnyIdent> "{" <e:Expr> "}" <r:@R>
@@ -207,12 +213,15 @@ Relation: Node<Option<cst::Relation>> = {
         => Node::with_source_loc(Some(cst::Relation::Common{initial: i, extended: e}), Loc::new(l..r, Arc::clone(src))),
     <l:@L> <t:Add> HAS <f:Add> <r:@R>
         => Node::with_source_loc(Some(cst::Relation::Has{target: t, field: f}), Loc::new(l..r, Arc::clone(src))),
-    <l:@L> <t:Add> HAS IF <r:@R> => {
+    // The following rule exists allegedly for the sake of better error
+    // reporting. RFC 62 (extended has operator) allows a sequence of
+    // identifiers separated by . as RHS. Hence, we need to extend this rule to
+    // `HAS IF { MemAccess }`, as opposed to the original `HAS IF`.
+    <l:@L> <t:Add> HAS <ii:IfIdent> <a:MemAccess*> <r:@R> => {
         // Create an add expression from this identifier
-        let id0 = Node::with_source_loc(Some(cst::Ident::If), Loc::new(l..r, Arc::clone(src)));
-        let id1 = Node::with_source_loc(Some(cst::Name{path: vec![], name: id0}), Loc::new(l..r, Arc::clone(src)));
+        let id1 = Node::with_source_loc(Some(cst::Name{path: vec![], name: ii}), Loc::new(l..r, Arc::clone(src)));
         let id2 = Node::with_source_loc(Some(cst::Primary::Name(id1)), Loc::new(l..r, Arc::clone(src)));
-        let id3 = Node::with_source_loc(Some(cst::Member{ item: id2, access: vec![] }), Loc::new(l..r, Arc::clone(src)));
+        let id3 = Node::with_source_loc(Some(cst::Member{ item: id2, access: a }), Loc::new(l..r, Arc::clone(src)));
         let id4 = Node::with_source_loc(Some(cst::Unary{op: None, item:id3}), Loc::new(l..r, Arc::clone(src)));
         let id5 = Node::with_source_loc(Some(cst::Mult{initial: id4, extended: vec![]}), Loc::new(l..r, Arc::clone(src)));
         let id6 = Node::with_source_loc(Some(cst::Add{initial:id5, extended: vec![]}), Loc::new(l..r, Arc::clone(src)));