Skip to content

Commit

Permalink
Ensure __expr is an error
Browse files Browse the repository at this point in the history
  • Loading branch information
@aaronjeline
aaronjeline committed Oct 9, 2023
1 parent e07dd04 commit 26690f3
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 0 deletions.
164 changes: 164 additions & 0 deletions cedar-policy-core/src/entities.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,8 @@ pub enum TCComputation {
}

#[cfg(test)]
// PANIC SAFETY unit tests
#[allow(clippy::panic)]
mod json_parsing_tests {

use super::*;
Expand Down Expand Up @@ -986,6 +988,164 @@ mod json_parsing_tests {
)
}

#[test]
fn no_expr_escapes1() {
let json = serde_json::json!(
[
{
"uid" : r#"test_entity_type::"Alice""#,
"attrs": {
"bacon": "eggs",
"pancakes": [1, 2, 3],
"waffles": { "key": "value" },
"toast" : { "__extn" : { "fn" : "decimal", "arg" : "33.47" }},
"12345": { "__entity": { "type": "test_entity_type", "id": "bob" } },
"a b c": { "__extn": { "fn": "ip", "arg": "222.222.222.0/24" } }
},
"parents": [
{ "__entity": { "type" : "test_entity_type", "id" : "bob"} },
{ "__entity": { "type": "test_entity_type", "id": "catherine" } }
]
},
]);
let eparser: EntityJsonParser<'_> =
EntityJsonParser::new(None, Extensions::all_available(), TCComputation::ComputeNow);
let error = eparser.from_json_value(json).err().unwrap().to_string();
assert!(
error.contains("in uid field of <unknown entity>, expected a literal entity reference"),
"{}",
error
);
}

#[test]
fn no_expr_escapes2() {
let json = serde_json::json!(
[
{
"uid" : {
"__expr" :
r#"test_entity_type::"Alice""#
},
"attrs": {
"bacon": "eggs",
"pancakes": [1, 2, 3],
"waffles": { "key": "value" },
"toast" : { "__extn" : { "fn" : "decimal", "arg" : "33.47" }},
"12345": { "__entity": { "type": "test_entity_type", "id": "bob" } },
"a b c": { "__extn": { "fn": "ip", "arg": "222.222.222.0/24" } }
},
"parents": [
{ "__entity": { "type" : "test_entity_type", "id" : "bob"} },
{ "__entity": { "type": "test_entity_type", "id": "catherine" } }
]
}
]);
let eparser: EntityJsonParser<'_> =
EntityJsonParser::new(None, Extensions::all_available(), TCComputation::ComputeNow);
let error = eparser.from_json_value(json).err().unwrap().to_string();
assert!(
error.contains("in uid field of <unknown entity>, expected a literal entity reference"),
"{}",
error
);
}

#[test]
fn no_expr_escapes3() {
let json = serde_json::json!(
[
{
"uid" : {
"type" : "test_entity_type",
"id" : "Alice"
},
"attrs": {
"bacon": "eggs",
"pancakes": { "__expr" : "[1,2,3]" },
"waffles": { "key": "value" },
"toast" : { "__extn" : { "fn" : "decimal", "arg" : "33.47" }},
"12345": { "__entity": { "type": "test_entity_type", "id": "bob" } },
"a b c": { "__extn": { "fn": "ip", "arg": "222.222.222.0/24" } }
},
"parents": [
{ "__entity": { "type" : "test_entity_type", "id" : "bob"} },
{ "__entity": { "type": "test_entity_type", "id": "catherine" } }
]
}
]);
let eparser: EntityJsonParser<'_> =
EntityJsonParser::new(None, Extensions::all_available(), TCComputation::ComputeNow);
let error = eparser.from_json_value(json).err().unwrap().to_string();
assert!(
error.contains("`__expr` tag is no longer supported"),
"Actual error message was: {}",
error
);
}

#[test]
fn no_expr_escapes4() {
let json = serde_json::json!(
[
{
"uid" : {
"type" : "test_entity_type",
"id" : "Alice"
},
"attrs": {
"bacon": "eggs",
"waffles": { "key": "value" },
"12345": { "__entity": { "type": "test_entity_type", "id": "bob" } },
"a b c": { "__extn": { "fn": "ip", "arg": "222.222.222.0/24" } }
},
"parents": [
{ "__expr": { "type" : "test_entity_type", "id" : "bob"} },
{ "__entity": { "type": "test_entity_type", "id": "catherine" } }
]
}
]);
let eparser: EntityJsonParser<'_> =
EntityJsonParser::new(None, Extensions::all_available(), TCComputation::ComputeNow);
let error = eparser.from_json_value(json).err().unwrap().to_string();
assert!(
error.contains(r#"in parents field of `test_entity_type::"Alice"`, expected a literal entity reference"#),
"Actual error message was: {}",
error
);
}

#[test]
fn no_expr_escapes5() {
let json = serde_json::json!(
[
{
"uid" : {
"type" : "test_entity_type",
"id" : "Alice"
},
"attrs": {
"bacon": "eggs",
"waffles": { "key": "value" },
"12345": { "__entity": { "type": "test_entity_type", "id": "bob" } },
"a b c": { "__extn": { "fn": "ip", "arg": "222.222.222.0/24" } }
},
"parents": [
"test_entity_type::\"bob\"",
{ "__entity": { "type": "test_entity_type", "id": "catherine" } }
]
}
]);
let eparser: EntityJsonParser<'_> =
EntityJsonParser::new(None, Extensions::all_available(), TCComputation::ComputeNow);
let error = eparser.from_json_value(json).err().unwrap().to_string();
assert!(
error.contains(r#"in parents field of `test_entity_type::"Alice"`, expected a literal entity reference"#),
"Actual error message was: {}",
error
);
}

#[cfg(feature = "ipaddr")]
/// this one uses `__entity` and `__extn` escapes, in various positions
#[test]
Expand Down Expand Up @@ -1419,6 +1579,8 @@ mod json_parsing_tests {
}

#[cfg(test)]
// PANIC SAFETY unit tests
#[allow(clippy::panic)]
mod entities_tests {
use super::*;

Expand Down Expand Up @@ -1491,6 +1653,8 @@ mod entities_tests {
}

#[cfg(test)]
// PANIC SAFETY unit tests
#[allow(clippy::panic)]
mod schema_based_parsing_tests {
use super::*;
use crate::extensions::Extensions;
Expand Down
3 changes: 3 additions & 0 deletions cedar-policy-core/src/entities/json/err.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,9 @@ pub enum JsonDeserializationError {
/// Parent type which was invalid
parent_ty: Box<EntityType>, // boxed to avoid this variant being very large (and thus all JsonDeserializationErrors being large)
},
#[error("{0}, Invalid tag. The `__expr` tag is no longer supported")]
/// Raised when a JsonValue contains the no longer support `__expr` tag
ExprTag(Box<JsonDeserializationErrorContext>),
}

/// Errors thrown during serialization to JSON
Expand Down
8 changes: 8 additions & 0 deletions cedar-policy-core/src/entities/json/jsonvalue.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ use std::collections::{HashMap, HashSet};
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)]
#[serde(untagged)]
pub enum JSONValue {
/// `__expr` is deprecated, but still throws an error was present.
ExprEscape {
/// Contents, will be ignored and an error is thrown are attempting to parse this
__expr: SmolStr,
},
/// Special JSON object with single reserved "__entity" key:
/// the following item should be a JSON object of the form
/// `{ "type": "xxx", "id": "yyy" }`.
Expand Down Expand Up @@ -161,6 +166,9 @@ impl JSONValue {
})?,
)),
Self::ExtnEscape { __extn: extn } => extn.into_expr(),
Self::ExprEscape { .. } => Err(JsonDeserializationError::ExprTag(Box::new(
JsonDeserializationErrorContext::Context,
))),
}
}

Expand Down
2 changes: 2 additions & 0 deletions cedar-policy-validator/src/err.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ pub enum SchemaError {
/// This error variant should only be used when `PermitAttributes` is enabled.
#[error("action `{0}` has an attribute with unsupported JSON representation: {1}")]
UnsupportedActionAttribute(EntityUID, String),
#[error("uses the `__expr` escape, which is no longer supported")]
ExprEscapeUsed,
}

impl From<transitive_closure::TcError<EntityUID>> for SchemaError {
Expand Down
1 change: 1 addition & 0 deletions cedar-policy-validator/src/schema.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ impl ValidatorNamespaceDef {
action_id.clone(),
"extension function escape (`__extn`)".to_owned(),
)),
JSONValue::ExprEscape { .. } => Err(SchemaError::ExprEscapeUsed),
}
}

Expand Down
5 changes: 5 additions & 0 deletions cedar-policy/src/api.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,10 @@ pub enum SchemaError {
/// This error variant should only be used when `PermitAttributes` is enabled.
#[error("action `{0}` has an attribute with unsupported JSON representation: {1}")]
UnsupportedActionAttribute(EntityUid, String),
/// Error thrown when the schema contains the `__expr` escape.
/// Support for this escape form has been dropped.
#[error("schema contained the non-supported `__expr` escape.")]
ExprEscapeUsed,
}

/// Describes in what action context or entity type shape a schema parsing error
Expand Down Expand Up @@ -1190,6 +1194,7 @@ impl From<cedar_policy_validator::SchemaError> for SchemaError {
cedar_policy_validator::SchemaError::UnsupportedActionAttribute(uid, escape_type) => {
Self::UnsupportedActionAttribute(EntityUid(uid), escape_type)
}
cedar_policy_validator::SchemaError::ExprEscapeUsed => Self::ExprEscapeUsed,
}
}
}
Expand Down

0 comments on commit 26690f3

Please sign in to comment.