Skip to content

Commit f11d0c1

Browse files
Fix validation error msg for invalid attribute on nested entity attribute (#811)
Signed-off-by: John Kastner <[email protected]>
1 parent c7ff260 commit f11d0c1

File tree

2 files changed

+34
-5
lines changed

2 files changed

+34
-5
lines changed

cedar-policy-validator/src/type_error.rs

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -440,11 +440,13 @@ pub(crate) enum AttributeAccess {
440440
}
441441

442442
impl AttributeAccess {
443+
/// Construct an `AttributeAccess` access from a `GetAttr` expression `expr.attr`.
443444
pub(crate) fn from_expr(
444445
req_env: &RequestEnv,
445446
mut expr: &Expr<Option<Type>>,
447+
attr: SmolStr,
446448
) -> AttributeAccess {
447-
let mut attrs: Vec<SmolStr> = Vec::new();
449+
let mut attrs: Vec<SmolStr> = vec![attr];
448450
loop {
449451
if let Some(Type::EntityOrRecord(EntityRecordKind::Entity(lub))) = expr.data() {
450452
return AttributeAccess::EntityLUB(lub.clone(), attrs);
@@ -525,7 +527,7 @@ impl Display for AttributeAccess {
525527
// optional attribute without a guard, then the help message is also printed.
526528
#[cfg(test)]
527529
mod test_attr_access {
528-
use cedar_policy_core::ast::{EntityType, EntityUID, Expr, ExprBuilder, Var};
530+
use cedar_policy_core::ast::{EntityType, EntityUID, Expr, ExprBuilder, ExprKind, Var};
529531

530532
use crate::{
531533
types::{OpenTag, RequestEnv, Type},
@@ -548,7 +550,11 @@ mod test_attr_access {
548550
resource_slot: None,
549551
};
550552

551-
let access = AttributeAccess::from_expr(&env, attr_access);
553+
let ExprKind::GetAttr { expr, attr } = attr_access.expr_kind() else {
554+
panic!("Can only test `AttributeAccess::from_expr` for `GetAttr` expressions");
555+
};
556+
557+
let access = AttributeAccess::from_expr(&env, expr, attr.clone());
552558
assert_eq!(
553559
access.to_string().as_str(),
554560
msg.as_ref(),
@@ -603,6 +609,21 @@ mod test_attr_access {
603609
);
604610
}
605611

612+
#[test]
613+
fn entity_type_attr_access() {
614+
let e = ExprBuilder::with_data(Some(Type::named_entity_reference_from_str("Thing")))
615+
.get_attr(
616+
ExprBuilder::with_data(Some(Type::named_entity_reference_from_str("User")))
617+
.var(Var::Principal),
618+
"thing".into(),
619+
);
620+
assert_message_and_help(&e, "`thing` for entity type User", "e has thing");
621+
let e = ExprBuilder::new().get_attr(e, "bar".into());
622+
assert_message_and_help(&e, "`bar` for entity type Thing", "e has bar");
623+
let e = ExprBuilder::new().get_attr(e, "baz".into());
624+
assert_message_and_help(&e, "`bar.baz` for entity type Thing", "e.bar has baz");
625+
}
626+
606627
#[test]
607628
fn other_access() {
608629
let e = ExprBuilder::new().get_attr(

cedar-policy-validator/src/typecheck.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -999,7 +999,11 @@ impl<'a> Typechecker<'a> {
999999
} else {
10001000
type_errors.push(TypeError::unsafe_optional_attribute_access(
10011001
e.clone(),
1002-
AttributeAccess::from_expr(request_env, &annot_expr),
1002+
AttributeAccess::from_expr(
1003+
request_env,
1004+
&typ_expr_actual,
1005+
attr.clone(),
1006+
),
10031007
));
10041008
TypecheckAnswer::fail(annot_expr)
10051009
}
@@ -1023,7 +1027,11 @@ impl<'a> Typechecker<'a> {
10231027
let suggestion = fuzzy_search(attr, &borrowed);
10241028
type_errors.push(TypeError::unsafe_attribute_access(
10251029
e.clone(),
1026-
AttributeAccess::from_expr(request_env, &annot_expr),
1030+
AttributeAccess::from_expr(
1031+
request_env,
1032+
&typ_expr_actual,
1033+
attr.clone(),
1034+
),
10271035
suggestion,
10281036
Type::may_have_attr(self.schema, typ_actual, attr),
10291037
));

0 commit comments

Comments
 (0)