Entity Slicing: Type-annotated entity manifests

cedar-policy-validator/src/entity_manifest.rs

@@ -51,14 +51,10 @@ use crate::{ValidationResult, Validator};
 #[serde_as]
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
-pub struct EntityManifest<T = ()>
-where
-    T: Clone,
-{
+pub struct EntityManifest {
     /// A map from request types to [`RootAccessTrie`]s.
     #[serde_as(as = "Vec<(_, _)>")]
-    #[serde(bound(deserialize = "T: Default"))]
-    pub(crate) per_action: HashMap<RequestType, RootAccessTrie<T>>,
+    pub(crate) per_action: HashMap<RequestType, RootAccessTrie>,
 }
 
 /// A map of data fields to [`AccessTrie`]s.
@@ -68,7 +64,7 @@ where
 // Don't make fields `pub`, don't make breaking changes, and use caution
 // when adding public methods.
 #[doc = include_str!("../../cedar-policy/experimental_warning.md")]
-pub type Fields<T> = HashMap<SmolStr, Box<AccessTrie<T>>>;
+pub type Fields = HashMap<SmolStr, Box<AccessTrie>>;
 
 /// The root of a data path or [`RootAccessTrie`].
 // CAUTION: this type is publicly exported in `cedar-policy`.
@@ -109,14 +105,10 @@ impl Display for EntityRoot {
 #[serde_as]
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
-pub struct RootAccessTrie<T = ()>
-where
-    T: Clone,
-{
+pub struct RootAccessTrie {
     /// The data that needs to be loaded, organized by root.
     #[serde_as(as = "Vec<(_, _)>")]
-    #[serde(bound(deserialize = "T: Default"))]
-    pub(crate) trie: HashMap<EntityRoot, AccessTrie<T>>,
+    pub(crate) trie: HashMap<EntityRoot, AccessTrie>,
 }
 
 /// A Trie representing a set of data paths to load,
@@ -132,11 +124,11 @@ where
 #[serde_as]
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
-pub struct AccessTrie<T = ()> {
+pub struct AccessTrie {
     /// Child data of this entity slice.
     /// The keys are edges in the trie pointing to sub-trie values.
     #[serde_as(as = "Vec<(_, _)>")]
-    pub(crate) children: Fields<T>,
+    pub(crate) children: Fields,
     /// `ancestors_trie` is another [`RootAccessTrie`] representing
     /// all of the ancestors of this entity that are required.
     /// The ancestors trie is a subset of the original [`RootAccessTrie`].
@@ -147,10 +139,12 @@ pub struct AccessTrie<T = ()> {
     /// An ancestor trie can be thought of as a set of pointers to
     /// nodes in the original trie, one `is_ancestor`-marked node per pointer.
     pub(crate) is_ancestor: bool,
-    /// Optional data annotation, usually used for type information.
-    #[serde(skip_serializing, skip_deserializing)]
-    #[serde(bound(deserialize = "T: Default"))]
-    pub(crate) data: T,
+    /// The type of this node in the [`AccessTrie`].
+    /// From the public API, this field should always be `Some`.
+    /// It is `None` after deserialization or after first being constructed, but it is type annotated right away.
+    #[serde(skip_serializing)]
+    #[serde(skip_deserializing)]
+    pub(crate) node_type: Option<Type>,
 }
 
 /// An access path represents path of fields, starting with an [`EntityRoot`].
@@ -204,16 +198,90 @@ pub enum EntityManifestError {
     PartialExpression(#[from] PartialExpressionError),
 }
 
-impl<T: Clone> EntityManifest<T> {
+/// Error when the manifest has an entity the schema lacks.
+// CAUTION: this type is publicly exported in `cedar-policy`.
+// Don't make fields `pub`, don't make breaking changes, and use caution
+// when adding public methods.
+#[derive(Debug, Clone, Error, Hash, Eq, PartialEq)]
+#[error("entity manifest doesn't match schema. Schema is missing entity {entity}. Either you wrote an entity manifest by hand (not reccomended) or you are using an out-of-date entity manifest with respect to the schema.")]
+pub struct MismatchedMissingEntityError {
+    pub(crate) entity: EntityUID,
+}
+
+/// Error when the schema isn't valid in strict mode.
+// CAUTION: this type is publicly exported in `cedar-policy`.
+// Don't make fields `pub`, don't make breaking changes, and use caution
+// when adding public methods.
+#[derive(Debug, Clone, Error, Hash, Eq, PartialEq)]
+#[error("entity manifests are only compatible with schemas that validate in strict mode. Tried to use an invalid schema with an entity manifest")]
+pub struct MismatchedNotStrictSchemaError {}
+
+/// An error generated by entity manifest parsing. These happen
+/// when the entity manifest doesn't conform to the schema.
+/// Either the user wrote an entity manifest by hand (not reccomended)
+/// or they used an out-of-date entity manifest (after updating the schema).
+/// Warning: This error is not guaranteed to happen, even when an entity
+/// manifest is out-of-date with respect to a schema! Users must ensure
+/// that entity manifests are in-sync with the schema and policies.
+// CAUTION: this type is publicly exported in `cedar-policy`.
+// Don't make fields `pub`, don't make breaking changes, and use caution
+// when adding public methods.
+#[derive(Debug, Clone, Error, Hash, Eq, PartialEq)]
+pub enum MismatchedEntityManifestError {
+    /// Mismatch between entity in manifest and schema
+    #[error(transparent)]
+    MismatchedMissingEntity(#[from] MismatchedMissingEntityError),
+    /// Found a schema that isn't valid in strict mode
+    #[error(transparent)]
+    MismatchedNotStrictSchema(#[from] MismatchedNotStrictSchemaError),
+}
+
+/// An error generated when parsing entity manifests from json
+#[derive(Debug, Error)]
+pub enum EntityManifestFromJsonError {
+    /// A Serde error happened
+    #[error(transparent)]
+    SerdeJsonParseError(#[from] serde_json::Error),
+    /// A mismatched entity manifest error
+    #[error(transparent)]
+    MismatchedEntityManifest(#[from] MismatchedEntityManifestError),
+}
+
+impl EntityManifest {
     /// Get the contents of the entity manifest
     /// indexed by the type of the request.
-    pub fn per_action(&self) -> &HashMap<RequestType, RootAccessTrie<T>> {
+    pub fn per_action(&self) -> &HashMap<RequestType, RootAccessTrie> {
         &self.per_action
     }
+
+    /// Convert a json string to an [`EntityManifest`].
+    /// Requires the schema in order to add type annotations.
+    pub fn from_json_str(
+        json: &str,
+        schema: &ValidatorSchema,
+    ) -> Result<Self, EntityManifestFromJsonError> {
+        match serde_json::from_str::<EntityManifest>(json) {
+            Ok(manifest) => manifest.to_typed(schema).map_err(|e| e.into()),
+            Err(e) => Err(e.into()),
+        }
+    }
+
+    /// Convert a json value to an [`EntityManifest`].
+    /// Requires the schema in order to add type annotations.
+    #[allow(dead_code)]
+    pub fn from_json_value(
+        value: serde_json::Value,
+        schema: &ValidatorSchema,
+    ) -> Result<Self, EntityManifestFromJsonError> {
+        match serde_json::from_value::<EntityManifest>(value) {
+            Ok(manifest) => manifest.to_typed(schema).map_err(|e| e.into()),
+            Err(e) => Err(e.into()),
+        }
+    }
 }
 
 /// Union two tries by combining the fields.
-fn union_fields<T: Clone>(first: &Fields<T>, second: &Fields<T>) -> Fields<T> {
+fn union_fields(first: &Fields, second: &Fields) -> Fields {
     let mut res = first.clone();
     for (key, value) in second {
         res.entry(key.clone())
@@ -245,7 +313,7 @@ impl AccessPath {
                 ancestors_trie: Default::default(),
                 is_ancestor: false,
                 children: fields,
-                data: (),
+                node_type: None,
             };
         }
 
@@ -260,10 +328,10 @@ impl AccessPath {
     }
 }
 
-impl<T: Clone> RootAccessTrie<T> {
+impl RootAccessTrie {
     /// Get the trie as a hash map from [`EntityRoot`]
     /// to sub-[`AccessTrie`]s.
-    pub fn trie(&self) -> &HashMap<EntityRoot, AccessTrie<T>> {
+    pub fn trie(&self) -> &HashMap<EntityRoot, AccessTrie> {
         &self.trie
     }
 }
@@ -277,7 +345,7 @@ impl RootAccessTrie {
     }
 }
 
-impl<T: Clone> RootAccessTrie<T> {
+impl RootAccessTrie {
     /// Union two [`RootAccessTrie`]s together.
     /// The new trie requests the data from both of the original.
     pub fn union(mut self, other: &Self) -> Self {
@@ -302,7 +370,7 @@ impl Default for RootAccessTrie {
     }
 }
 
-impl<T: Clone> AccessTrie<T> {
+impl AccessTrie {
     /// Union two [`AccessTrie`]s together.
     /// The new trie requests the data from both of the original.
     pub fn union(mut self, other: &Self) -> Self {
@@ -318,7 +386,7 @@ impl<T: Clone> AccessTrie<T> {
     }
 
     /// Get the children of this [`AccessTrie`].
-    pub fn children(&self) -> &Fields<T> {
+    pub fn children(&self) -> &Fields {
         &self.children
     }
 
@@ -327,22 +395,16 @@ impl<T: Clone> AccessTrie<T> {
     pub fn ancestors_required(&self) -> &RootAccessTrie {
         &self.ancestors_trie
     }
-
-    /// Get the data associated with this [`AccessTrie`].
-    /// This is usually `()` unless it is annotated by a type.
-    pub fn data(&self) -> &T {
-        &self.data
-    }
 }
 
 impl AccessTrie {
     /// A new trie that requests no data.
-    pub fn new() -> Self {
+    pub(crate) fn new() -> Self {
         Self {
             children: Default::default(),
             ancestors_trie: Default::default(),
             is_ancestor: false,
-            data: (),
+            node_type: None,
         }
     }
 }
@@ -403,9 +465,13 @@ pub fn compute_entity_manifest(
         }
     }
 
+    // PANIC SAFETY: entity manifest cannot be out of date, since it was computed from the schema given
+    #[allow(clippy::unwrap_used)]
     Ok(EntityManifest {
         per_action: manifest,
-    })
+    }
+    .to_typed(schema)
+    .unwrap())
 }
 
 /// A static analysis on type-annotated cedar expressions.
@@ -634,6 +700,40 @@ when {
         let schema = schema();
 
         let entity_manifest = compute_entity_manifest(&schema, &pset).expect("Should succeed");
+        let expected_rust = EntityManifest {
+            per_action: [(
+                RequestType {
+                    principal: "User".parse().unwrap(),
+                    resource: "Document".parse().unwrap(),
+                    action: "Action::\"Read\"".parse().unwrap(),
+                },
+                RootAccessTrie {
+                    trie: [(
+                        EntityRoot::Var(Var::Principal),
+                        AccessTrie {
+                            children: [(
+                                SmolStr::new("name"),
+                                Box::new(AccessTrie {
+                                    children: HashMap::new(),
+                                    ancestors_trie: RootAccessTrie::new(),
+                                    is_ancestor: false,
+                                    node_type: Some(Type::primitive_string()),
+                                }),
+                            )]
+                            .into_iter()
+                            .collect(),
+                            ancestors_trie: RootAccessTrie::new(),
+                            is_ancestor: false,
+                            node_type: Some(Type::named_entity_reference("User".parse().unwrap())),
+                        },
+                    )]
+                    .into_iter()
+                    .collect(),
+                },
+            )]
+            .into_iter()
+            .collect(),
+        };
         let expected = serde_json::json! ({
           "perAction": [
             [
@@ -671,8 +771,9 @@ when {
             ]
           ]
         });
-        let expected_manifest = serde_json::from_value(expected).unwrap();
+        let expected_manifest = EntityManifest::from_json_value(expected, &schema).unwrap();
         assert_eq!(entity_manifest, expected_manifest);
+        assert_eq!(entity_manifest, expected_rust);
     }
 
     #[test]
@@ -704,7 +805,7 @@ when {
             ]
           ]
         });
-        let expected_manifest = serde_json::from_value(expected).unwrap();
+        let expected_manifest = EntityManifest::from_json_value(expected, &schema).unwrap();
         assert_eq!(entity_manifest, expected_manifest);
     }
 
@@ -803,7 +904,7 @@ action Read appliesTo {
             ]
           ]
         });
-        let expected_manifest = serde_json::from_value(expected).unwrap();
+        let expected_manifest = EntityManifest::from_json_value(expected, &schema).unwrap();
         assert_eq!(entity_manifest, expected_manifest);
     }
 
@@ -915,7 +1016,7 @@ action Read appliesTo {
             ]
           ]
             });
-        let expected_manifest = serde_json::from_value(expected).unwrap();
+        let expected_manifest = EntityManifest::from_json_value(expected, &schema).unwrap();
         assert_eq!(entity_manifest, expected_manifest);
     }
 
@@ -1032,7 +1133,7 @@ action Read appliesTo {
             ]
           ]
         });
-        let expected_manifest = serde_json::from_value(expected).unwrap();
+        let expected_manifest = EntityManifest::from_json_value(expected, &schema).unwrap();
         assert_eq!(entity_manifest, expected_manifest);
     }
 
@@ -1132,7 +1233,7 @@ action BeSad appliesTo {
             ]
           ]
         });
-        let expected_manifest = serde_json::from_value(expected).unwrap();
+        let expected_manifest = EntityManifest::from_json_value(expected, &schema).unwrap();
         assert_eq!(entity_manifest, expected_manifest);
     }
 
@@ -1265,7 +1366,7 @@ action Hello appliesTo {
             ]
           ]
         });
-        let expected_manifest = serde_json::from_value(expected).unwrap();
+        let expected_manifest = EntityManifest::from_json_value(expected, &schema).unwrap();
         assert_eq!(entity_manifest, expected_manifest);
     }
 
@@ -1384,7 +1485,7 @@ when {
           ]
         }
         );
-        let expected_manifest = serde_json::from_value(expected).unwrap();
+        let expected_manifest = EntityManifest::from_json_value(expected, &schema).unwrap();
         assert_eq!(entity_manifest, expected_manifest);
     }
 
@@ -1489,7 +1590,7 @@ when {
           ]
         }
         );
-        let expected_manifest = serde_json::from_value(expected).unwrap();
+        let expected_manifest = EntityManifest::from_json_value(expected, &schema).unwrap();
         assert_eq!(entity_manifest, expected_manifest);
     }
 }

cedar-policy-validator/src/entity_manifest_analysis.rs

@@ -281,7 +281,7 @@ fn entity_or_record_to_access_trie(ty: &EntityRecordKind) -> AccessTrie {
                 children: fields,
                 ancestors_trie: Default::default(),
                 is_ancestor: false,
-                data: (),
+                node_type: None,
             }
         }