Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions cedar-policy-validator/src/type_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,11 +440,13 @@ pub(crate) enum AttributeAccess {
}

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

use crate::{
types::{OpenTag, RequestEnv, Type},
Expand All @@ -548,7 +550,11 @@ mod test_attr_access {
resource_slot: None,
};

let access = AttributeAccess::from_expr(&env, attr_access);
let ExprKind::GetAttr { expr, attr } = attr_access.expr_kind() else {
panic!("Can only test `AttributeAccess::from_expr` for `GetAttr` expressions");
};

let access = AttributeAccess::from_expr(&env, expr, attr.clone());
assert_eq!(
access.to_string().as_str(),
msg.as_ref(),
Expand Down Expand Up @@ -603,6 +609,21 @@ mod test_attr_access {
);
}

#[test]
fn entity_type_attr_access() {
let e = ExprBuilder::with_data(Some(Type::named_entity_reference_from_str("Thing")))
.get_attr(
ExprBuilder::with_data(Some(Type::named_entity_reference_from_str("User")))
.var(Var::Principal),
"thing".into(),
);
assert_message_and_help(&e, "`thing` for entity type User", "e has thing");
let e = ExprBuilder::new().get_attr(e, "bar".into());
assert_message_and_help(&e, "`bar` for entity type Thing", "e has bar");
let e = ExprBuilder::new().get_attr(e, "baz".into());
assert_message_and_help(&e, "`bar.baz` for entity type Thing", "e.bar has baz");
}

#[test]
fn other_access() {
let e = ExprBuilder::new().get_attr(
Expand Down
12 changes: 10 additions & 2 deletions cedar-policy-validator/src/typecheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,11 @@ impl<'a> Typechecker<'a> {
} else {
type_errors.push(TypeError::unsafe_optional_attribute_access(
e.clone(),
AttributeAccess::from_expr(request_env, &annot_expr),
AttributeAccess::from_expr(
request_env,
&typ_expr_actual,
attr.clone(),
),
));
TypecheckAnswer::fail(annot_expr)
}
Expand All @@ -1024,7 +1028,11 @@ impl<'a> Typechecker<'a> {
let suggestion = fuzzy_search(attr, &borrowed);
type_errors.push(TypeError::unsafe_attribute_access(
e.clone(),
AttributeAccess::from_expr(request_env, &annot_expr),
AttributeAccess::from_expr(
request_env,
&typ_expr_actual,
attr.clone(),
),
suggestion,
Type::may_have_attr(self.schema, typ_actual, attr),
));
Expand Down
6 changes: 6 additions & 0 deletions cedar-policy/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Validation error for invalid use of an action now includes a source location
containing the offending policy.

### Fixed

- Validation error message for an invalid attribute access now reports the
correct attribute and entity type when accessing an optional attribute that is
itself an entity.

## [3.1.3] - 2024-04-15

### Changed
Expand Down