Skip to content

Commit

Permalink
Merge branch 'main' into entity_removal
Browse files Browse the repository at this point in the history
  • Loading branch information
chaluli authored Feb 6, 2025
2 parents ae5d3b2 + 33bd811 commit 3e76529
Show file tree
Hide file tree
Showing 30 changed files with 1,903 additions and 456 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cedar-policy-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ smol_str = { version = "0.3", features = ["serde"] }
stacker = "0.1.15"
arbitrary = { version = "1", features = ["derive"], optional = true }
miette = { version = "7.5.0", features = ["serde"] }
nonempty = "0.10.0"
nonempty = { version = "0.10.0", features = ["serialize"] }
educe = "0.6.0"

# decimal extension requires regex
Expand Down
110 changes: 110 additions & 0 deletions cedar-policy-core/src/entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2163,6 +2163,7 @@ mod schema_based_parsing_tests {
use crate::extensions::Extensions;
use crate::test_utils::*;
use cool_asserts::assert_matches;
use nonempty::NonEmpty;
use serde_json::json;
use smol_str::SmolStr;
use std::collections::HashSet;
Expand Down Expand Up @@ -2253,6 +2254,9 @@ mod schema_based_parsing_tests {
/// Mock schema impl for the `Employee` type used in most of these tests
struct MockEmployeeDescription;
impl EntityTypeDescription for MockEmployeeDescription {
fn enum_entity_eids(&self) -> Option<NonEmpty<Eid>> {
None
}
fn entity_type(&self) -> EntityType {
EntityType::from(Name::parse_unqualified_name("Employee").expect("valid"))
}
Expand Down Expand Up @@ -3502,6 +3506,9 @@ mod schema_based_parsing_tests {

struct MockEmployeeDescription;
impl EntityTypeDescription for MockEmployeeDescription {
fn enum_entity_eids(&self) -> Option<NonEmpty<Eid>> {
None
}
fn entity_type(&self) -> EntityType {
"XYZCorp::Employee".parse().expect("valid")
}
Expand Down Expand Up @@ -3630,6 +3637,109 @@ mod schema_based_parsing_tests {
);
});
}

#[test]
fn enumerated_entities() {
struct MockSchema;
struct StarTypeDescription;
impl EntityTypeDescription for StarTypeDescription {
fn entity_type(&self) -> EntityType {
"Star".parse().unwrap()
}

fn attr_type(&self, _attr: &str) -> Option<SchemaType> {
None
}

fn tag_type(&self) -> Option<SchemaType> {
None
}

fn required_attrs<'s>(&'s self) -> Box<dyn Iterator<Item = SmolStr> + 's> {
Box::new(std::iter::empty())
}

fn allowed_parent_types(&self) -> Arc<HashSet<EntityType>> {
Arc::new(HashSet::new())
}

fn open_attributes(&self) -> bool {
false
}

fn enum_entity_eids(&self) -> Option<NonEmpty<Eid>> {
Some(nonempty::nonempty![Eid::new("🌎"), Eid::new("🌕"),])
}
}
impl Schema for MockSchema {
type EntityTypeDescription = StarTypeDescription;

type ActionEntityIterator = std::iter::Empty<Arc<Entity>>;

fn entity_type(&self, entity_type: &EntityType) -> Option<Self::EntityTypeDescription> {
if entity_type == &"Star".parse::<EntityType>().unwrap() {
Some(StarTypeDescription)
} else {
None
}
}

fn action(&self, _action: &EntityUID) -> Option<Arc<Entity>> {
None
}

fn entity_types_with_basename<'a>(
&'a self,
basename: &'a UnreservedId,
) -> Box<dyn Iterator<Item = EntityType> + 'a> {
if basename == &"Star".parse::<UnreservedId>().unwrap() {
Box::new(std::iter::once("Star".parse::<EntityType>().unwrap()))
} else {
Box::new(std::iter::empty())
}
}

fn action_entities(&self) -> Self::ActionEntityIterator {
std::iter::empty()
}
}

let eparser = EntityJsonParser::new(
Some(&MockSchema),
Extensions::none(),
TCComputation::ComputeNow,
);

assert_matches!(
eparser.from_json_value(serde_json::json!([
{
"uid": { "type": "Star", "id": "🌎" },
"attrs": {},
"parents": [],
}
])),
Ok(_)
);

let entitiesjson = serde_json::json!([
{
"uid": { "type": "Star", "id": "🪐" },
"attrs": {},
"parents": [],
}
]);
assert_matches!(eparser.from_json_value(entitiesjson.clone()),
Err(e) => {
expect_err(
&entitiesjson,
&miette::Report::new(e),
&ExpectedErrorMessageBuilder::error("entity does not conform to the schema")
.source(r#"entity `Star::"🪐"` is of an enumerated entity type, but `"🪐"` is not declared as a valid eid"#)
.help(r#"valid entity eids: "🌎", "🌕""#)
.build()
);
});
}
}

#[cfg(feature = "protobufs")]
Expand Down
68 changes: 67 additions & 1 deletion cedar-policy-core/src/entities/conformance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use std::collections::BTreeMap;

use super::{json::err::TypeMismatchError, EntityTypeDescription, Schema, SchemaType};
use super::{Eid, EntityUID, Literal};
use crate::ast::{
BorrowedRestrictedExpr, Entity, PartialValue, PartialValueToRestrictedExprError, RestrictedExpr,
};
Expand All @@ -27,7 +28,7 @@ use smol_str::SmolStr;
use thiserror::Error;
pub mod err;

use err::{EntitySchemaConformanceError, UnexpectedEntityTypeError};
use err::{EntitySchemaConformanceError, InvalidEnumEntityError, UnexpectedEntityTypeError};

/// Struct used to check whether entities conform to a schema
#[derive(Debug, Clone)]
Expand Down Expand Up @@ -61,6 +62,8 @@ impl<'a, S: Schema> EntitySchemaConformanceChecker<'a, S> {
));
}
} else {
validate_euid(self.schema, uid)
.map_err(|e| EntitySchemaConformanceError::InvalidEnumEntity(e.into()))?;
let schema_etype = self.schema.entity_type(etype).ok_or_else(|| {
let suggested_types = self
.schema
Expand Down Expand Up @@ -120,10 +123,14 @@ impl<'a, S: Schema> EntitySchemaConformanceChecker<'a, S> {
}
}
}
validate_euids_in_partial_value(self.schema, val)
.map_err(|e| EntitySchemaConformanceError::InvalidEnumEntity(e.into()))?;
}
// For each ancestor that actually appears in `entity`, ensure the
// ancestor type is allowed by the schema
for ancestor_euid in entity.ancestors() {
validate_euid(self.schema, ancestor_euid)
.map_err(|e| EntitySchemaConformanceError::InvalidEnumEntity(e.into()))?;
let ancestor_type = ancestor_euid.entity_type();
if schema_etype.allowed_parent_types().contains(ancestor_type) {
// note that `allowed_parent_types()` was transitively
Expand All @@ -137,11 +144,70 @@ impl<'a, S: Schema> EntitySchemaConformanceChecker<'a, S> {
));
}
}

for (_, val) in entity.tags() {
validate_euids_in_partial_value(self.schema, val)
.map_err(|e| EntitySchemaConformanceError::InvalidEnumEntity(e.into()))?;
}
}
Ok(())
}
}

/// Return an [`InvalidEnumEntityError`] if `uid`'s eid is not among valid `choices`
pub fn is_valid_enumerated_entity(
choices: &[Eid],
uid: &EntityUID,
) -> Result<(), InvalidEnumEntityError> {
choices
.iter()
.find(|id| uid.eid() == *id)
.ok_or(InvalidEnumEntityError {
uid: uid.clone(),
choices: choices.to_vec(),
})
.map(|_| ())
}

/// Validate if `euid` is valid, provided that it's of an enumerated type
pub(crate) fn validate_euid(
schema: &impl Schema,
euid: &EntityUID,
) -> Result<(), InvalidEnumEntityError> {
if let Some(desc) = schema.entity_type(euid.entity_type()) {
if let Some(choices) = desc.enum_entity_eids() {
is_valid_enumerated_entity(&Vec::from(choices), euid)?;
}
}
Ok(())
}

fn validate_euids_in_subexpressions<'a>(
exprs: impl Iterator<Item = &'a crate::ast::Expr>,
schema: &impl Schema,
) -> std::result::Result<(), InvalidEnumEntityError> {
exprs
.map(|e| match e.expr_kind() {
ExprKind::Lit(Literal::EntityUID(euid)) => validate_euid(schema, &euid),
_ => Ok(()),
})
.collect::<std::result::Result<(), _>>()
}

/// Validate if enumerated entities in `val` are valid
pub fn validate_euids_in_partial_value(
schema: &impl Schema,
val: &PartialValue,
) -> Result<(), InvalidEnumEntityError> {
match val {
PartialValue::Value(val) => validate_euids_in_subexpressions(
RestrictedExpr::from(val.clone()).subexpressions(),
schema,
),
PartialValue::Residual(e) => validate_euids_in_subexpressions(e.subexpressions(), schema),
}
}

/// Check whether the given `PartialValue` typechecks with the given `SchemaType`.
/// If the typecheck passes, return `Ok(())`.
/// If the typecheck fails, return an appropriate `Err`.
Expand Down
50 changes: 49 additions & 1 deletion cedar-policy-core/src/entities/conformance/err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
*/
//! This module cotnains errors around entities not conforming to schemas
use super::TypeMismatchError;
use crate::ast::{EntityType, EntityUID};
use crate::ast::{Eid, EntityType, EntityUID};
use crate::extensions::ExtensionFunctionLookupError;
use crate::impl_diagnostic_from_method_on_field;
use itertools::Itertools;
use miette::Diagnostic;
use smol_str::SmolStr;
use thiserror::Error;
Expand Down Expand Up @@ -70,6 +72,10 @@ pub enum EntitySchemaConformanceError {
#[error(transparent)]
#[diagnostic(transparent)]
ExtensionFunctionLookup(ExtensionFunctionLookup),
/// Returned when an entity is of an enumerated entity type but has invalid EID
#[error(transparent)]
#[diagnostic(transparent)]
InvalidEnumEntity(#[from] InvalidEnumEntity),
}

impl EntitySchemaConformanceError {
Expand Down Expand Up @@ -277,3 +283,45 @@ impl Diagnostic for UnexpectedEntityTypeError {
}
}
}

/// Returned when an entity is of an enumerated entity type but has invalid EID
//
// 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, Error, Diagnostic)]
#[error(transparent)]
#[diagnostic(transparent)]
pub struct InvalidEnumEntity {
err: InvalidEnumEntityError,
}

impl From<InvalidEnumEntityError> for InvalidEnumEntity {
fn from(value: InvalidEnumEntityError) -> Self {
Self { err: value }
}
}

/// Returned when an entity is of an enumerated entity type but has invalid EID
#[derive(Debug, Error, Clone, PartialEq, Eq, Hash)]
#[error("entity `{uid}` is of an enumerated entity type, but `\"{}\"` is not declared as a valid eid", uid.eid().escaped())]
pub struct InvalidEnumEntityError {
/// Entity where the error occurred
pub uid: EntityUID,
/// Name of the attribute where the error occurred
pub choices: Vec<Eid>,
}

impl Diagnostic for InvalidEnumEntityError {
impl_diagnostic_from_method_on_field!(uid, loc);

fn help<'a>(&'a self) -> Option<Box<dyn std::fmt::Display + 'a>> {
Some(Box::new(format!(
"valid entity eids: {}",
self.choices
.iter()
.map(|e| format!("\"{}\"", e.escaped()))
.join(", ")
)))
}
}
10 changes: 9 additions & 1 deletion cedar-policy-core/src/entities/json/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
*/

use super::SchemaType;
use crate::ast::{Entity, EntityType, EntityUID};
use crate::ast::{Eid, Entity, EntityType, EntityUID};
use crate::entities::{Name, UnreservedId};
use nonempty::NonEmpty;
use smol_str::SmolStr;
use std::collections::HashSet;
use std::sync::Arc;
Expand Down Expand Up @@ -137,6 +138,10 @@ pub trait EntityTypeDescription {
/// May entities with this type have attributes other than those specified
/// in the schema
fn open_attributes(&self) -> bool;

/// Return valid EID choices if the entity type is enumerated otherwise
/// return `None`
fn enum_entity_eids(&self) -> Option<NonEmpty<Eid>>;
}

/// Simple type that implements `EntityTypeDescription` by expecting no
Expand Down Expand Up @@ -165,6 +170,9 @@ impl EntityTypeDescription for NullEntityTypeDescription {
fn open_attributes(&self) -> bool {
false
}
fn enum_entity_eids(&self) -> Option<NonEmpty<Eid>> {
None
}
}
impl NullEntityTypeDescription {
/// Create a new [`NullEntityTypeDescription`] for the given entity typename
Expand Down
1 change: 1 addition & 0 deletions cedar-policy-validator/protobuf_schema/Validator.proto
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ message ValidatorEntityType {
Attributes attributes = 3;
OpenTag open_attributes = 4;
Tag tags = 5;
repeated string enums = 6;
}

message ValidatorActionId {
Expand Down
Loading

0 comments on commit 3e76529

Please sign in to comment.