fix accepting tags as an attr name (#1209)

Signed-off-by: Craig Disselkoen <[email protected]>

Author: cdisselkoen

cedar-policy-core/src/parser.rs

@@ -621,6 +621,18 @@ mod tests {
         assert!(!result.expect("parse error").is_empty());
     }
 
+    #[test]
+    fn attr_named_tags() {
+        let src = r#"
+            permit(principal, action, resource)
+            when {
+                resource.tags.contains({k: "foo", v: "bar"})
+            };
+        "#;
+        parse_policy_to_est_and_ast(None, src)
+            .unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)));
+    }
+
     #[test]
     fn test_parse_policyset() {
         use crate::ast::PolicyID;

cedar-policy-validator/src/cedar_schema/err.rs

@@ -72,6 +72,7 @@ lazy_static! {
             ("TYPE", "`type`"),
             ("SET", "`Set`"),
             ("IDENTIFIER", "identifier"),
+            ("TAGS", "`tags`"),
         ]),
         impossible_tokens: HashSet::new(),
         special_identifier_tokens: HashSet::from([
@@ -85,6 +86,7 @@ lazy_static! {
             "RESOURCE",
             "CONTEXT",
             "ATTRIBUTES",
+            "TAGS",
             "LONG",
             "STRING",
             "BOOL",

cedar-policy-validator/src/cedar_schema/grammar.lalrpop

@@ -61,14 +61,14 @@ match {
     "entity" => ENTITY,
     "in" => IN,
     "type" => TYPE,
-    "tags" => TAGS,
     "Set" => SET,
     "appliesTo" => APPLIESTO,
     "principal" => PRINCIPAL,
     "action" => ACTION,
     "resource" => RESOURCE,
     "context" => CONTEXT,
     "attributes" => ATTRIBUTES,
+    "tags" => TAGS,
     "Long" => LONG,
     "String" => STRING,
     "Bool" => BOOL,
@@ -220,6 +220,8 @@ Ident: Node<Id> = {
         => Node::with_source_loc("context".parse().unwrap(), Loc::new(l..r, Arc::clone(src))),
     <l:@L> ATTRIBUTES <r:@R>
         => Node::with_source_loc("attributes".parse().unwrap(), Loc::new(l..r, Arc::clone(src))),
+    <l:@L> TAGS <r:@R>
+        => Node::with_source_loc("tags".parse().unwrap(), Loc::new(l..r, Arc::clone(src))),
     <l:@L> BOOL <r:@R>
         => Node::with_source_loc("Bool".parse().unwrap(), Loc::new(l..r, Arc::clone(src))),
     <l:@L> LONG <r:@R>

cedar-policy-validator/src/cedar_schema/test.rs

@@ -35,7 +35,7 @@ mod demo_tests {
             err::{ToJsonSchemaError, NO_PR_HELP_MSG},
         },
         json_schema,
-        schema::test::collect_warnings,
+        schema::test::utils::collect_warnings,
         CedarSchemaError, RawName,
     };
 
@@ -1168,7 +1168,7 @@ mod translator_tests {
             to_json_schema::cedar_schema_to_json_schema,
         },
         json_schema,
-        schema::test::collect_warnings,
+        schema::test::utils::collect_warnings,
         types::{EntityLUB, EntityRecordKind, Primitive, Type},
         ValidatorSchema,
     };
@@ -2302,7 +2302,7 @@ mod common_type_references {
 #[cfg(test)]
 mod entity_tags {
     use crate::json_schema;
-    use crate::schema::test::collect_warnings;
+    use crate::schema::test::utils::collect_warnings;
     use cedar_policy_core::extensions::Extensions;
     use cool_asserts::assert_matches;
 

cedar-policy-validator/src/schema.rs

@@ -1328,27 +1328,68 @@ pub(crate) mod test {
 
     use super::*;
 
-    /// Transform the output of functions like
-    /// `ValidatorSchema::from_cedarschema_str()`, which has type `(ValidatorSchema, impl Iterator<...>)`,
-    /// into `(ValidatorSchema, Vec<...>)`, which implements `Debug` and thus can be used with
-    /// `assert_matches`, `.unwrap_err()`, etc
-    pub fn collect_warnings<A, B, E>(
-        r: std::result::Result<(A, impl Iterator<Item = B>), E>,
-    ) -> std::result::Result<(A, Vec<B>), E> {
-        r.map(|(a, iter)| (a, iter.collect()))
-    }
-
-    /// Given an entity type as string, get the `ValidatorEntityType` from the
-    /// schema, panicking if it does not exist (or if `etype` fails to parse as
-    /// an entity type)
-    #[track_caller]
-    pub fn assert_entity_type_exists<'s>(
-        schema: &'s ValidatorSchema,
-        etype: &str,
-    ) -> &'s ValidatorEntityType {
-        schema.get_entity_type(&etype.parse().unwrap()).unwrap()
+    pub(crate) mod utils {
+        use super::{CedarSchemaError, SchemaError, ValidatorEntityType, ValidatorSchema};
+        use cedar_policy_core::extensions::Extensions;
+
+        /// Transform the output of functions like
+        /// `ValidatorSchema::from_cedarschema_str()`, which has type `(ValidatorSchema, impl Iterator<...>)`,
+        /// into `(ValidatorSchema, Vec<...>)`, which implements `Debug` and thus can be used with
+        /// `assert_matches`, `.unwrap_err()`, etc
+        pub fn collect_warnings<A, B, E>(
+            r: std::result::Result<(A, impl Iterator<Item = B>), E>,
+        ) -> std::result::Result<(A, Vec<B>), E> {
+            r.map(|(a, iter)| (a, iter.collect()))
+        }
+
+        /// Given an entity type as string, get the `ValidatorEntityType` from the
+        /// schema, panicking if it does not exist (or if `etype` fails to parse as
+        /// an entity type)
+        #[track_caller]
+        pub fn assert_entity_type_exists<'s>(
+            schema: &'s ValidatorSchema,
+            etype: &str,
+        ) -> &'s ValidatorEntityType {
+            schema.get_entity_type(&etype.parse().unwrap()).unwrap()
+        }
+
+        #[track_caller]
+        pub fn assert_valid_cedar_schema(src: &str) -> ValidatorSchema {
+            match ValidatorSchema::from_cedarschema_str(src, Extensions::all_available()) {
+                Ok((schema, _)) => schema,
+                Err(e) => panic!("{:?}", miette::Report::new(e)),
+            }
+        }
+
+        #[track_caller]
+        pub fn assert_invalid_cedar_schema(src: &str) {
+            match ValidatorSchema::from_cedarschema_str(src, Extensions::all_available()) {
+                Ok(_) => panic!("{src} should be an invalid schema"),
+                Err(CedarSchemaError::Parsing(_)) => {}
+                Err(e) => panic!("unexpected error: {:?}", miette::Report::new(e)),
+            }
+        }
+
+        #[track_caller]
+        pub fn assert_valid_json_schema(json: serde_json::Value) -> ValidatorSchema {
+            match ValidatorSchema::from_json_value(json, Extensions::all_available()) {
+                Ok(schema) => schema,
+                Err(e) => panic!("{:?}", miette::Report::new(e)),
+            }
+        }
+
+        #[track_caller]
+        pub fn assert_invalid_json_schema(json: serde_json::Value) {
+            match ValidatorSchema::from_json_value(json.clone(), Extensions::all_available()) {
+                Ok(_) => panic!("{json} should be an invalid schema"),
+                Err(SchemaError::JsonDeserialization(_)) => {}
+                Err(e) => panic!("unexpected error: {:?}", miette::Report::new(e)),
+            }
+        }
     }
 
+    use utils::*;
+
     // Well-formed schema
     #[test]
     fn test_from_schema_file() {
@@ -3443,15 +3484,23 @@ pub(crate) mod test {
             );
         });
     }
+
+    #[test]
+    fn attr_named_tags() {
+        let src = r#"
+            entity E { tags: Set<{key: String, value: Set<String>}> };
+        "#;
+        assert_valid_cedar_schema(src);
+    }
 }
 
 #[cfg(test)]
 mod test_579; // located in separate file test_579.rs
 
 #[cfg(test)]
 mod test_rfc70 {
-    use super::test::{assert_entity_type_exists, collect_warnings};
-    use super::{CedarSchemaError, SchemaError, ValidatorSchema};
+    use super::test::utils::*;
+    use super::ValidatorSchema;
     use crate::types::Type;
     use cedar_policy_core::{
         extensions::Extensions,
@@ -3460,40 +3509,6 @@ mod test_rfc70 {
     use cool_asserts::assert_matches;
     use serde_json::json;
 
-    #[track_caller]
-    fn assert_valid_cedar_schema(src: &str) -> ValidatorSchema {
-        match ValidatorSchema::from_cedarschema_str(src, Extensions::all_available()) {
-            Ok((schema, _)) => schema,
-            Err(e) => panic!("{:?}", miette::Report::new(e)),
-        }
-    }
-
-    #[track_caller]
-    fn assert_invalid_cedar_schema(src: &str) {
-        match ValidatorSchema::from_cedarschema_str(src, Extensions::all_available()) {
-            Ok(_) => panic!("{src} should be an invalid schema"),
-            Err(CedarSchemaError::Parsing(_)) => {}
-            Err(e) => panic!("unexpected error: {:?}", miette::Report::new(e)),
-        }
-    }
-
-    #[track_caller]
-    fn assert_valid_json_schema(json: serde_json::Value) -> ValidatorSchema {
-        match ValidatorSchema::from_json_value(json, Extensions::all_available()) {
-            Ok(schema) => schema,
-            Err(e) => panic!("{:?}", miette::Report::new(e)),
-        }
-    }
-
-    #[track_caller]
-    fn assert_invalid_json_schema(json: serde_json::Value) {
-        match ValidatorSchema::from_json_value(json.clone(), Extensions::all_available()) {
-            Ok(_) => panic!("{json} should be an invalid schema"),
-            Err(SchemaError::JsonDeserialization(_)) => {}
-            Err(e) => panic!("unexpected error: {:?}", miette::Report::new(e)),
-        }
-    }
-
     /// Common type shadowing a common type is disallowed in both syntaxes
     #[test]
     fn common_common_conflict() {
@@ -4476,10 +4491,7 @@ mod test_rfc70 {
 mod entity_tags {
     use crate::types::Primitive;
 
-    use super::{
-        test::{assert_entity_type_exists, collect_warnings},
-        *,
-    };
+    use super::{test::utils::*, *};
     use cedar_policy_core::{
         extensions::Extensions,
         test_utils::{expect_err, ExpectedErrorMessageBuilder},

cedar-policy-validator/src/schema/test_579.rs

@@ -60,7 +60,7 @@
 //! we only do the test for the more sensible one. (For instance, for 1a1, we
 //! only test an entity type reference, not a common type reference.)
 
-use super::{test::collect_warnings, SchemaWarning, ValidatorSchema};
+use super::{test::utils::collect_warnings, SchemaWarning, ValidatorSchema};
 use cedar_policy_core::extensions::Extensions;
 use cedar_policy_core::test_utils::{
     expect_err, ExpectedErrorMessage, ExpectedErrorMessageBuilder,