diff --git a/cedar-policy-cli/src/command/tpe.rs b/cedar-policy-cli/src/command/tpe.rs index 93460c288..109d99d9f 100644 --- a/cedar-policy-cli/src/command/tpe.rs +++ b/cedar-policy-cli/src/command/tpe.rs @@ -216,7 +216,7 @@ pub fn tpe(args: &TpeArgs) -> CedarExitCode { None => { println!("UNKNOWN"); println!("All policy residuals:"); - for p in ans.residual_policies() { + for p in ans.policies() { println!("{p}"); } CedarExitCode::Unknown diff --git a/cedar-policy-core/src/pst/expr.rs b/cedar-policy-core/src/pst/expr.rs index d57a2c07b..e32c7661c 100644 --- a/cedar-policy-core/src/pst/expr.rs +++ b/cedar-policy-core/src/pst/expr.rs @@ -984,7 +984,11 @@ impl Expr { /// Does this expression contain any [`Expr::ResidualError`] nodes? /// /// Returns `true` if any subexpression represents a statically-known - /// evaluation error (as determined by TPE). + /// evaluation error (as determined by TPE). Note that this says nothing + /// about whether the expression will, or even can, evaluate to an error. + /// This function should only be used to decide if the expression can be + /// represented as valid Cedar policy text, since the error residual variant + /// does not exist in the Cedar policy syntax. #[cfg(feature = "tpe")] pub fn has_error(&self) -> bool { self.reduce::( diff --git a/cedar-policy-core/src/tpe.rs b/cedar-policy-core/src/tpe.rs index 655e7babf..4cfd8f8bf 100644 --- a/cedar-policy-core/src/tpe.rs +++ b/cedar-policy-core/src/tpe.rs @@ -106,11 +106,11 @@ pub fn is_authorized<'a>( mod tests { use cool_asserts::assert_matches; - use crate::ast::{Annotation, AnyId, BinaryOp, Literal, Value, ValueKind, Var}; + use crate::ast::{Annotation, AnyId, BinaryOp, Literal, PolicyID, Value, ValueKind, Var}; use crate::tpe::residual::{Residual, ResidualKind}; use crate::validator::ValidatorSchema; use crate::{ - ast::{Eid, EntityUID, PolicySet}, + ast::{EntityUID, PolicySet}, extensions::Extensions, parser::parse_policyset, }; @@ -126,6 +126,73 @@ mod tests { use super::is_authorized; + pub(super) fn collect_policy_ids<'a>( + iter: impl Iterator, + ) -> HashSet<&'a PolicyID> { + iter.map(|p| p.get_policy_id()).collect() + } + + #[track_caller] + pub(super) fn get_policy_by_annotation_id<'a>( + ps: &'a PolicySet, + annotation_id: &str, + ) -> &'a crate::ast::Policy { + let id_key = AnyId::new_unchecked("id"); + ps.static_policies() + .find(|p| { + matches!(p.annotation(&id_key), Some(Annotation { val, .. }) if val == annotation_id) + }) + .unwrap() + } + + #[track_caller] + pub(super) fn assert_resource_get_attr(residual: &Residual, expected_attr: &str) { + assert_matches!( + residual, + Residual::Partial { + kind: ResidualKind::GetAttr { expr, attr }, + .. + } => { + assert_matches!( + expr.as_ref(), + Residual::Partial { kind: ResidualKind::Var(Var::Resource), .. } + ); + assert_eq!(attr, expected_attr); + } + ); + } + + #[track_caller] + pub(super) fn assert_in_resource_attr( + residual: &Residual, + expected_uid: &EntityUID, + expected_attr: &str, + ) { + assert_matches!( + residual, + Residual::Partial { + kind: ResidualKind::BinaryApp { op: BinaryOp::In, arg1, arg2 }, + .. + } => { + assert_entity_uid_literal(arg1.as_ref(), expected_uid); + assert_resource_get_attr(arg2.as_ref(), expected_attr); + } + ); + } + + #[track_caller] + pub(super) fn assert_entity_uid_literal(residual: &Residual, expected_uid: &EntityUID) { + assert_matches!( + residual, + Residual::Concrete { + value: Value { value: ValueKind::Lit(Literal::EntityUID(uid)), .. }, + .. + } => { + assert_eq!(uid.as_ref(), expected_uid); + } + ); + } + fn rfc_policies() -> PolicySet { parse_policyset( r#" @@ -202,11 +269,8 @@ action Delete appliesTo { fn rfc_request() -> PartialRequest { PartialRequest { - principal: PartialEntityUID { - ty: "User".parse().unwrap(), - eid: Some(Eid::new("Alice")), - }, - action: EntityUID::from_components("Action".parse().unwrap(), Eid::new("View"), None), + principal: r#"User::"Alice""#.parse::().unwrap().into(), + action: r#"Action::"View""#.parse().unwrap(), resource: PartialEntityUID { ty: "Document".parse().unwrap(), eid: None, @@ -219,7 +283,7 @@ action Delete appliesTo { } fn rfc_entities() -> PartialEntities { - let uid = EntityUID::from_components("User".parse().unwrap(), Eid::new("Alice"), None); + let uid: EntityUID = r#"User::"Alice""#.parse().unwrap(); PartialEntities::from_entities_unchecked( [( uid.clone(), @@ -240,44 +304,49 @@ action Delete appliesTo { let request = rfc_request(); let entities = rfc_entities(); let residuals = is_authorized(&policies, &request, &entities, &schema).unwrap(); - let id = AnyId::new_unchecked("id"); - let policy0 = policies - .static_policies() - .find(|p| matches!(p.annotation(&id), Some(Annotation {val, ..}) if val == "0")) - .unwrap(); - let policy1 = policies - .static_policies() - .find(|p| matches!(p.annotation(&id), Some(Annotation {val, ..}) if val == "1")) - .unwrap(); - let policy2 = policies - .static_policies() - .find(|p| matches!(p.annotation(&id), Some(Annotation {val, ..}) if val == "2")) - .unwrap(); - // resource["isPublic"] - assert_matches!(residuals.get_residual(policy0.id()), Some(Residual::Partial{kind: ResidualKind::GetAttr { expr, attr }, ..}) => { - assert_matches!(expr.as_ref(), Residual::Partial { kind: ResidualKind::Var(Var::Resource), .. }); - assert_eq!(attr, "isPublic"); - }); - // (resource["owner"]) == User::"Alice" - assert_matches!(residuals.get_residual(policy1.id()), Some(Residual::Partial { kind: ResidualKind::BinaryApp { op: BinaryOp::Eq, arg1, arg2 }, .. }) => { - assert_matches!(arg1.as_ref(), Residual::Partial { kind: ResidualKind::GetAttr { expr, attr }, .. } => { - assert_matches!(expr.as_ref(), Residual::Partial { kind: ResidualKind::Var(Var::Resource), .. }); - assert_eq!(attr, "owner"); - }); - assert_matches!(arg2.as_ref(), Residual::Concrete { value: Value { value: ValueKind::Lit(Literal::EntityUID(uid)), ..}, .. } => { - assert_eq!(uid.as_ref(), &EntityUID::from_components("User".parse().unwrap(), Eid::new("Alice"), None)); - }); - }); - // false + + let policy0 = get_policy_by_annotation_id(&policies, "0"); + let policy1 = get_policy_by_annotation_id(&policies, "1"); + let policy2 = get_policy_by_annotation_id(&policies, "2"); + + // Policy 0: resource["isPublic"] + let r0 = residuals + .get_residual_policy(policy0.id()) + .unwrap() + .get_residual(); + assert_resource_get_attr(r0.as_ref(), "isPublic"); + + // Policy 1: resource["owner"] == User::"Alice" + let r1 = residuals + .get_residual_policy(policy1.id()) + .unwrap() + .get_residual(); + let alice: EntityUID = r#"User::"Alice""#.parse().unwrap(); + assert_matches!( + r1.as_ref(), + Residual::Partial { + kind: ResidualKind::BinaryApp { op: BinaryOp::Eq, arg1, arg2 }, + .. + } => { + assert_resource_get_attr(arg1.as_ref(), "owner"); + assert_entity_uid_literal(arg2.as_ref(), &alice); + } + ); + + // Policy 2: false (action doesn't match) + let r2 = residuals + .get_residual_policy(policy2.id()) + .unwrap() + .get_residual(); assert_matches!( - residuals.get_residual(policy2.id()), - Some(Residual::Concrete { + r2.as_ref(), + Residual::Concrete { value: Value { value: ValueKind::Lit(Literal::Bool(false)), .. }, .. - }) + } ); } } @@ -287,16 +356,10 @@ mod tinytodo { use std::collections::HashSet; use std::{collections::BTreeMap, sync::Arc}; - use crate::ast::{ - Annotation, AnyId, BinaryOp, EntityUID, Literal, PolicyID, Value, ValueKind, Var, - }; + use crate::ast::{BinaryOp, EntityUID}; use crate::tpe::residual::{Residual, ResidualKind}; use crate::validator::ValidatorSchema; - use crate::{ - ast::{Eid, PolicySet}, - extensions::Extensions, - parser::parse_policyset, - }; + use crate::{ast::PolicySet, extensions::Extensions, parser::parse_policyset}; use cool_asserts::assert_matches; use serde_json::json; @@ -306,6 +369,10 @@ mod tinytodo { }; use super::is_authorized; + use super::tests::{ + assert_entity_uid_literal, assert_in_resource_attr, assert_resource_get_attr, + collect_policy_ids, get_policy_by_annotation_id, + }; #[track_caller] fn schema() -> ValidatorSchema { @@ -403,10 +470,7 @@ when { principal in resource.editors }; #[track_caller] fn partial_request() -> PartialRequest { PartialRequest { - principal: PartialEntityUID { - ty: "User".parse().unwrap(), - eid: Some(Eid::new("aaron")), - }, + principal: r#"User::"aaron""#.parse::().unwrap().into(), action: r#"Action::"GetList""#.parse().unwrap(), resource: PartialEntityUID { ty: "List".parse().unwrap(), @@ -440,88 +504,58 @@ when { principal in resource.editors }; let request = partial_request(); let entities = partial_entities(); let residuals = is_authorized(&policies, &request, &entities, &schema).unwrap(); - let id = AnyId::new_unchecked("id"); - let policy0 = policies - .static_policies() - .find(|p| matches!(p.annotation(&id), Some(Annotation {val, ..}) if val == "0")) - .unwrap(); - let policy1 = policies - .static_policies() - .find(|p| matches!(p.annotation(&id), Some(Annotation {val, ..}) if val == "1")) - .unwrap(); - let policy2 = policies - .static_policies() - .find(|p| matches!(p.annotation(&id), Some(Annotation {val, ..}) if val == "2")) - .unwrap(); - let policy3 = policies - .static_policies() - .find(|p| matches!(p.annotation(&id), Some(Annotation {val, ..}) if val == "3")) - .unwrap(); - let false_permits: HashSet<&PolicyID> = residuals - .false_permits() - .map(|p| p.get_policy_id()) - .collect(); - assert!(false_permits.len() == 2); - assert!(false_permits.contains(policy0.id())); - assert!(false_permits.contains(policy3.id())); - let false_forbids: HashSet<&PolicyID> = residuals - .false_forbids() - .map(|p| p.get_policy_id()) - .collect(); - assert!(false_forbids.is_empty()); - let true_permits: HashSet<&PolicyID> = residuals - .satisfied_permits() - .map(|p| p.get_policy_id()) - .collect(); - assert!(true_permits.is_empty()); - let true_forbids: HashSet<&PolicyID> = residuals - .satisfied_forbids() - .map(|p| p.get_policy_id()) - .collect(); - assert!(true_forbids.is_empty()); - let non_trivial_permits: HashSet<&PolicyID> = residuals - .residual_permits() - .map(|p| p.get_policy_id()) - .collect(); - assert!(non_trivial_permits.len() == 2); - assert!(non_trivial_permits.contains(policy1.id())); - assert!(non_trivial_permits.contains(policy2.id())); - let non_trivial_forbids: HashSet<&PolicyID> = residuals - .residual_forbids() - .map(|p| p.get_policy_id()) - .collect(); - assert!(non_trivial_forbids.is_empty()); + + let policy0 = get_policy_by_annotation_id(&policies, "0"); + let policy1 = get_policy_by_annotation_id(&policies, "1"); + let policy2 = get_policy_by_annotation_id(&policies, "2"); + let policy3 = get_policy_by_annotation_id(&policies, "3"); + + // Check response categorization + let false_permits = collect_policy_ids(residuals.false_permits()); + assert_eq!(false_permits, HashSet::from([policy0.id(), policy3.id()])); + assert!(collect_policy_ids(residuals.false_forbids()).is_empty()); + assert!(collect_policy_ids(residuals.true_permits()).is_empty()); + assert!(collect_policy_ids(residuals.true_forbids()).is_empty()); + + let residual_permits = collect_policy_ids(residuals.residual_permits()); + assert_eq!( + residual_permits, + HashSet::from([policy1.id(), policy2.id()]) + ); + assert!(collect_policy_ids(residuals.residual_forbids()).is_empty()); assert_matches!(residuals.decision(), None); - // (resource["owner"]) == User::"aaron" - assert_matches!(residuals.get_residual(policy1.id()), Some(Residual::Partial { kind: ResidualKind::BinaryApp { op: BinaryOp::Eq, arg1, arg2 }, .. }) => { - assert_matches!(arg1.as_ref(), Residual::Partial { kind: ResidualKind::GetAttr { expr, attr }, .. } => { - assert_matches!(expr.as_ref(), Residual::Partial { kind: ResidualKind::Var(Var::Resource), .. }); - assert_eq!(attr, "owner"); - }); - assert_matches!(arg2.as_ref(), Residual::Concrete { value: Value { value: ValueKind::Lit(Literal::EntityUID(uid)), ..}, .. } => { - assert_eq!(uid.as_ref(), &EntityUID::from_components("User".parse().unwrap(), Eid::new("aaron"), None)); - }); - }); - // (User::"aaron" in (resource["readers"])) || (User::"aaron" in (resource["editors"])) - assert_matches!(residuals.get_residual(policy2.id()), Some(Residual::Partial { kind: ResidualKind::Or{ left, right }, .. }) => { - assert_matches!(left.as_ref(), Residual::Partial { kind: ResidualKind::BinaryApp { op: BinaryOp::In, arg1, arg2 }, .. } => { - assert_matches!(arg1.as_ref(), Residual::Concrete { value: Value { value: ValueKind::Lit(Literal::EntityUID(uid)), ..}, .. } => { - assert_eq!(uid.as_ref(), &EntityUID::from_components("User".parse().unwrap(), Eid::new("aaron"), None)); - }); - assert_matches!(arg2.as_ref(), Residual::Partial { kind: ResidualKind::GetAttr { expr, attr }, .. } => { - assert_matches!(expr.as_ref(), Residual::Partial { kind: ResidualKind::Var(Var::Resource), .. }); - assert_eq!(attr, "readers"); - }); - }); - assert_matches!(right.as_ref(), Residual::Partial { kind: ResidualKind::BinaryApp { op: BinaryOp::In, arg1, arg2 }, .. } => { - assert_matches!(arg1.as_ref(), Residual::Concrete { value: Value { value: ValueKind::Lit(Literal::EntityUID(uid)), ..}, .. } => { - assert_eq!(uid.as_ref(), &EntityUID::from_components("User".parse().unwrap(), Eid::new("aaron"), None)); - }); - assert_matches!(arg2.as_ref(), Residual::Partial { kind: ResidualKind::GetAttr { expr, attr }, .. } => { - assert_matches!(expr.as_ref(), Residual::Partial { kind: ResidualKind::Var(Var::Resource), .. }); - assert_eq!(attr, "editors"); - }); - }); - }); + + // Policy 1: resource["owner"] == User::"aaron" + let aaron: EntityUID = r#"User::"aaron""#.parse().unwrap(); + let r1 = residuals + .get_residual_policy(policy1.id()) + .unwrap() + .get_residual(); + assert_matches!( + r1.as_ref(), + Residual::Partial { + kind: ResidualKind::BinaryApp { op: BinaryOp::Eq, arg1, arg2 }, + .. + } => { + assert_resource_get_attr(arg1.as_ref(), "owner"); + assert_entity_uid_literal(arg2.as_ref(), &aaron); + } + ); + + // Policy 2: (User::"aaron" in resource["readers"]) || (User::"aaron" in resource["editors"]) + let r2 = residuals + .get_residual_policy(policy2.id()) + .unwrap() + .get_residual(); + assert_matches!( + r2.as_ref(), + Residual::Partial { + kind: ResidualKind::Or { left, right }, + .. + } => { + assert_in_resource_attr(left.as_ref(), &aaron, "readers"); + assert_in_resource_attr(right.as_ref(), &aaron, "editors"); + } + ); } } diff --git a/cedar-policy-core/src/tpe/response.rs b/cedar-policy-core/src/tpe/response.rs index b78df1900..21c78505b 100644 --- a/cedar-policy-core/src/tpe/response.rs +++ b/cedar-policy-core/src/tpe/response.rs @@ -195,7 +195,7 @@ impl<'a> Response<'a> { } /// Get satisfied permit residual policies - pub fn satisfied_permits(&self) -> impl Iterator { + pub fn true_permits(&self) -> impl Iterator { #[expect( clippy::unwrap_used, reason = "we know that the policy ids are in the residuals map" @@ -206,7 +206,7 @@ impl<'a> Response<'a> { } /// Get satisfied forbid residual policies - pub fn satisfied_forbids(&self) -> impl Iterator { + pub fn true_forbids(&self) -> impl Iterator { #[expect( clippy::unwrap_used, reason = "we know that the policy ids are in the residuals map" @@ -282,9 +282,9 @@ impl<'a> Response<'a> { .map(|id| self.residuals.get(id).unwrap()) } - /// Look up the [`Residual`] by [`PolicyID`] - pub fn get_residual(&self, id: &PolicyID) -> Option<&Residual> { - self.residuals.get(id).map(|rp| rp.residual.as_ref()) + /// Look up the [`ResidualPolicy`] by [`PolicyID`] + pub fn get_residual_policy(&self, id: &PolicyID) -> Option<&ResidualPolicy> { + self.residuals.get(id) } /// Attempt to get the authorization decision @@ -318,16 +318,24 @@ impl<'a> Response<'a> { self.request.check_consistency(request)?; let authorizer = Authorizer::new(); - #[expect(clippy::unwrap_used, reason = "policy ids should not clash")] - Ok(authorizer.is_authorized( - request.clone(), - &PolicySet::try_from_iter(self.residuals.values().map(|rp| rp.clone().into())).unwrap(), - entities, - )) + Ok(authorizer.is_authorized(request.clone(), &self.policy_set(), entities)) } - /// Get residual policies - pub fn residual_policies(&self) -> impl Iterator { + /// Get all policies (including concrete true/false/error residuals) + pub fn policies(&self) -> impl Iterator { self.residuals.values() } + + /// Get all policies (including concrete true/false/error residuals) as a `PolicySet` + pub fn policy_set(&self) -> PolicySet { + let mut ps = PolicySet::new(); + for p in self.policies() { + #[expect( + clippy::unwrap_used, + reason = "`PolicySet::add` only fails on duplicate ids, but all residual policies will have unique ids" + )] + ps.add(p.policy.as_ref().clone()).unwrap() + } + ps + } } diff --git a/cedar-policy/CHANGELOG.md b/cedar-policy/CHANGELOG.md index e887330c4..d5e857dd8 100644 --- a/cedar-policy/CHANGELOG.md +++ b/cedar-policy/CHANGELOG.md @@ -19,11 +19,16 @@ Starting with version 3.2.4, changes marked with a star (*) are _language breaki ### Added - Public syntax tree (`pst`) support for `variadic-is-in-range` feature: a variadic `isInRange` is modelled by a `pst::Expr::VariadicOp{...}` in the PST (#2380). -- Functions for inspecting TPE policy evaluation results (`TpeResponse::reason` and iterators for true/false/error/residual permit/forbid policy IDs). +- For the experimental `tpe` feature, added functions for inspecting partial evaluation results. Adds `TpeResponse::reason` to get the ids for policies + contributing to the authorization decision, and specific iterators to list true/false/error/residual permit/forbid policy IDs. Also adds `TpeResponse::get_policy` + to lookup a partially evaluated policy by id, and `TpeResponse::policy_set` to retrieve all partial evaluated policies as a `PolicySet`. ### Changed - The experimental protobuf decoding API now validates its inputs, checking structural invariants on entities, expressions, templates, policy sets, and schemas. Additionally, `Entities::decode` now computes the transitive closure instead of assuming it is already computed. These changes may result in lower performance for protobuf decoding. The previous, unvalidated behavior is available via the new `decode_unchecked` methods (e.g., `Entities::decode_unchecked`) for trusted encoded data. +- For the experimental `tpe` feature, `TpeResponse::residual_policies` is updated to return only _non-trivial_ residuals and + `TpeResponse::nontrivial_residual_policies` is deprecated. The previous behavior (iterating all residuals including trivial ones) + is available via `TpeResponse::policies`. ## [4.11.2] - Coming Soon diff --git a/cedar-policy/src/api.rs b/cedar-policy/src/api.rs index 49d3681c0..67406b69d 100644 --- a/cedar-policy/src/api.rs +++ b/cedar-policy/src/api.rs @@ -2527,9 +2527,8 @@ impl AsRef for PolicySet { } #[doc(hidden)] -impl TryFrom for PolicySet { - type Error = PolicySetError; - fn try_from(pset: ast::PolicySet) -> Result { +impl From for PolicySet { + fn from(pset: ast::PolicySet) -> Self { Self::from_ast(pset) } } @@ -2626,7 +2625,7 @@ impl PolicySet { } /// Build the [`PolicySet`] from just the AST information - pub(crate) fn from_ast(ast: ast::PolicySet) -> Result { + pub(crate) fn from_ast(ast: ast::PolicySet) -> Self { let templates = ast .templates() .cloned() @@ -2637,11 +2636,11 @@ impl PolicySet { .cloned() .map(|p| (PolicyId::new(p.id().clone()), p.into())) .collect(); - Ok(Self { + Self { ast, policies, templates, - }) + } } /// Construct a [`PolicySet`] from a PST [`pst::PolicySet`]. diff --git a/cedar-policy/src/api/tpe.rs b/cedar-policy/src/api/tpe.rs index f91906edb..0c732bb7d 100644 --- a/cedar-policy/src/api/tpe.rs +++ b/cedar-policy/src/api/tpe.rs @@ -106,6 +106,8 @@ impl PartialRequest { } /// Like [`PartialRequest`] but only `resource` can be unknown +/// +/// Intended for use with [`PolicySet::query_resource`]. #[doc = include_str!("../../experimental_warning.md")] #[repr(transparent)] #[derive(Debug, Clone, RefCast)] @@ -130,38 +132,56 @@ impl ResourceQueryRequest { .map(Self) } - /// Convert [`ResourceQueryRequest`] to a [`Request`] by providing the resource [`EntityId`] + fn principal(&self) -> EntityUid { + #[expect( + clippy::unwrap_used, + reason = "constructor requires concrete principal" + )] + EntityUid(self.0 .0.get_principal().try_into().unwrap()) + } + + fn context(&self) -> Context { + #[expect(clippy::unwrap_used, reason = "constructor requires concrete context")] + let context_attrs = self.0 .0.get_context_attrs().unwrap(); + #[expect( + clippy::unwrap_used, + reason = "building context from BTreeMap iter, so no duplicates are possible" + )] + Context::from_pairs( + context_attrs + .iter() + .map(|(a, v)| (a.to_string(), RestrictedExpression(v.clone().into()))), + ) + .unwrap() + } + + /// Convert this to a [`Request`] by providing the resource [`EntityId`] + /// + /// Even though the partial request was already validated in [`ResourceQueryRequest::new`], + /// to ensure that the concrete request returned here is valid we still need to + /// check the resource entity id. If the resource has an enum entity type, + /// then its id must be one of the listed instances of that type. pub fn to_request( &self, resource_id: EntityId, schema: Option<&Schema>, ) -> Result { - #[expect( - clippy::unwrap_used, - reason = "various fields are validated through the constructor" - )] Request::new( - EntityUid(self.0 .0.get_principal().try_into().unwrap()), + self.principal(), EntityUid(self.0 .0.get_action()), EntityUid::from_type_name_and_id( EntityTypeName(self.0 .0.get_resource_type()), resource_id, ), - Context::from_pairs( - self.0 - .0 - .get_context_attrs() - .unwrap() - .iter() - .map(|(a, v)| (a.to_string(), RestrictedExpression(v.clone().into()))), - ) - .unwrap(), + self.context(), schema, ) } } /// Like [`PartialRequest`] but only `principal` can be unknown +/// +/// Intended for use with [`PolicySet::query_principal`]. #[doc = include_str!("../../experimental_warning.md")] #[repr(transparent)] #[derive(Debug, Clone, RefCast)] @@ -186,32 +206,45 @@ impl PrincipalQueryRequest { .map(Self) } - /// Convert [`PrincipalQueryRequest`] to a [`Request`] by providing the principal [`EntityId`] + fn resource(&self) -> EntityUid { + #[expect(clippy::unwrap_used, reason = "constructor requires concrete resource")] + EntityUid(self.0 .0.get_resource().try_into().unwrap()) + } + + fn context(&self) -> Context { + #[expect(clippy::unwrap_used, reason = "constructor requires concrete context")] + let context_attrs = self.0 .0.get_context_attrs().unwrap(); + #[expect( + clippy::unwrap_used, + reason = "building context from BTreeMap iter, so no duplicates are possible" + )] + Context::from_pairs( + context_attrs + .iter() + .map(|(a, v)| (a.to_string(), RestrictedExpression(v.clone().into()))), + ) + .unwrap() + } + + /// Convert this to a [`Request`] by providing the principal [`EntityId`] + /// + /// Even though the partial request was already validated in [`PrincipalQueryRequest::new`], + /// to ensure that the concrete request returned here is valid we still need to + /// check the principal entity id. If the principal has an enum entity type, + /// then its id must be one of the listed instances of that type. pub fn to_request( &self, principal_id: EntityId, schema: Option<&Schema>, ) -> Result { - #[expect( - clippy::unwrap_used, - reason = "various fields are validated through the constructor" - )] Request::new( EntityUid::from_type_name_and_id( EntityTypeName(self.0 .0.get_principal_type()), principal_id, ), EntityUid(self.0 .0.get_action()), - EntityUid(self.0 .0.get_resource().try_into().unwrap()), - Context::from_pairs( - self.0 - .0 - .get_context_attrs() - .unwrap() - .iter() - .map(|(a, v)| (a.to_string(), RestrictedExpression(v.clone().into()))), - ) - .unwrap(), + self.resource(), + self.context(), schema, ) } @@ -433,7 +466,7 @@ impl TpeResponse<'_> { /// policy. pub fn true_permits(&self) -> impl Iterator { self.0 - .satisfied_permits() + .true_permits() .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) } @@ -463,7 +496,7 @@ impl TpeResponse<'_> { /// Get the forbid policies that did not reach a concrete value or error for the partial request. /// /// This function only returns the `PolicyId`s for residual policies. - /// To access the residual policy conditions, use [`TpeResponse::nontrivial_residual_policies`]. + /// To access the residual policy conditions, use [`TpeResponse::residual_policies`]. /// /// The presence of any residual forbids means that [`TpeResponse::decision`] _cannot_ return /// a concrete `Allow` decision. We do not have enough information to say that these forbid @@ -481,7 +514,7 @@ impl TpeResponse<'_> { /// must return a concrete `Deny`. pub fn true_forbids(&self) -> impl Iterator { self.0 - .satisfied_forbids() + .true_forbids() .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) } @@ -526,20 +559,23 @@ impl TpeResponse<'_> { .map_err(Into::into) } - /// Return residuals as [`Policy`]s. + /// Returns an iterator of non-trivial (meaning more than just `true` + /// or `false`, or an error) residuals as [`Policy`]s. /// - /// Each returned [`Policy`] inherits its [`PolicyId`](crate::PolicyId) and + /// To find policies that reached a concrete value, use, e.g., [`TpeResponse::true_permits`]. + /// + /// Each returned [`Policy`] inherits its [`PolicyId`] and /// annotations from the corresponding input policy. Its scope is /// unconstrained and its condition is a single `when` clause containing /// the residual expression. /// - /// Use [`TpeResponse::nontrivial_residual_policies`] to skip trivially - /// `true` or `false` residuals. + /// Call [`Policy::to_pst()`] on each result to get a [`pst::Policy`](crate::pst::Policy) + /// for structured inspection. /// - /// The returned policies can be converted to PST for structured - /// inspection of the residual expression tree: - /// - /// ```text + /// ```no_run + /// # use cedar_policy::{PolicySet, PartialRequest, PartialEntities, Schema}; + /// # fn main() -> Result<(), Box> { + /// # let (policy_set, request, entities, schema) : (&PolicySet, &PartialRequest, &PartialEntities, &Schema) = panic!(); /// let response = policy_set.tpe(&request, &entities, &schema)?; /// for policy in response.residual_policies() { /// let pst_policy = policy.to_pst()?; @@ -547,29 +583,61 @@ impl TpeResponse<'_> { /// // inspect the residual expression via pst::Clause / pst::Expr /// } /// } + /// # Ok(()) + /// # } /// ``` + /// + /// When inspecting these policies, be aware that they may contain + /// [`pst::Expr::ResidualError`](crate::pst::Expr::ResidualError) nodes + /// which do not normally exist in Cedar expressions. These represent + /// subexpressions which are statically known to error; however, the whole + /// residual policy might or might not error, regardless of whether it + /// contains these nodes. pub fn residual_policies(&self) -> impl Iterator + '_ { self.0 - .residual_policies() + .residual_permits() + .chain(self.0.residual_forbids()) .map(|p| Policy::from_ast(p.clone().into())) } - /// Returns an iterator of non-trivial (meaning more than just `true` - /// or `false`) residuals as [`Policy`]s. + /// Return all residuals as [`Policy`]s, including concretely `true`, `false`, and error residuals. /// /// Each returned [`Policy`] inherits its [`PolicyId`](crate::PolicyId) and /// annotations from the corresponding input policy. Its scope is /// unconstrained and its condition is a single `when` clause containing /// the residual expression. /// - /// Call [`Policy::to_pst()`] on each result to get a [`crate::pst::Policy`] - /// for structured inspection. Residual expressions may contain - /// [`crate::pst::Expr::ResidualError`] nodes indicating subexpressions that - /// would error at runtime; use [`crate::pst::Expr::has_error()`] to check. + /// Use [`TpeResponse::residual_policies`] to skip `true`, `false`, and error residuals. + /// + /// See [`TpeResponse::residual_policies`] for documentation on how to inspect policies using the PST. + pub fn policies(&self) -> impl Iterator + '_ { + self.0 + .policies() + .map(|p| Policy::from_ast(p.clone().into())) + } + + /// Return all residuals as a [`PolicySet`], including concretely `true`, `false`, and error residuals. + /// + /// This returns exactly the same policies as [`TpeResponse::policies`], but collected into a policy set. + pub fn policy_set(&self) -> PolicySet { + PolicySet::from_ast(self.0.policy_set()) + } + + /// Deprecated alias for [`TpeResponse::residual_policies`] + #[deprecated( + since = "4.12.0", + note = "TpeResponse::residual_policies now returns only non-trivial residual policies" + )] pub fn nontrivial_residual_policies(&'_ self) -> impl Iterator + '_ { + self.residual_policies() + } + + /// Get the residual policy for a specific [`PolicyId`], if it exists. + /// + /// See [`TpeResponse::residual_policies`] for documentation on how to inspect policies using the PST. + pub fn get_policy(&self, id: &PolicyId) -> Option { self.0 - .residual_permits() - .chain(self.0.residual_forbids()) + .get_residual_policy(id.as_ref()) .map(|p| Policy::from_ast(p.clone().into())) } } @@ -638,11 +706,9 @@ impl PolicySet { /// Perform type-aware partial evaluation on this [`PolicySet`]. /// /// If successful, the result is a [`TpeResponse`] containing residual - /// policies ready for re-authorization. Use - /// [`TpeResponse::residual_policies()`] or - /// [`TpeResponse::nontrivial_residual_policies()`] to get the residuals - /// as [`Policy`] objects, then call [`Policy::to_pst()`] to convert them - /// to [`crate::pst::Policy`] for structured inspection of the residual + /// policies ready for re-authorization. Use [`TpeResponse::residual_policies()`] + /// to get the residuals as [`Policy`] objects, then call [`Policy::to_pst()`] + /// to convert them to [`crate::pst::Policy`] for structured inspection of the residual /// expression tree. #[doc = include_str!("../../experimental_warning.md")] pub fn tpe<'a>( @@ -691,27 +757,13 @@ impl PolicySet { schema: &Schema, ) -> Result, PermissionQueryError> { let partial_entities = PartialEntities::from_concrete(entities.clone(), schema)?; - let residuals = self.tpe(&request.0, &partial_entities, schema)?; - #[expect( - clippy::unwrap_used, - reason = "policy set construction should succeed because there shouldn't be any policy id conflicts" - )] - let policies = &Self::from_policies( - residuals - .0 - .residual_policies() - .map(|p| Policy::from_ast(p.clone().into())), - ) - .unwrap(); - #[expect( - clippy::unwrap_used, - reason = "request construction should succeed because each entity passes validation" - )] - match residuals.decision() { + let tpe_response = self.tpe(&request.0, &partial_entities, schema)?; + let policies = tpe_response.policy_set(); + match tpe_response.decision() { Some(Decision::Allow) => Ok(entities .iter() .filter(|entity| entity.0.uid().entity_type() == &request.0 .0.get_resource_type()) - .map(super::Entity::uid) + .map(Entity::uid) .collect_vec() .into_iter()), Some(Decision::Deny) => Ok(vec![].into_iter()), @@ -719,17 +771,24 @@ impl PolicySet { .iter() .filter(|entity| entity.0.uid().entity_type() == &request.0 .0.get_resource_type()) .filter(|entity| { + #[expect( + clippy::unwrap_used, reason = "`to_request` cannot panic because we do not pass a schema. However, the correctness of the authorization + decision depends on having valid a request and entities, but we do not do any validation here. Entities were already validated by + `PartialEntities::from_concrete`. The request was _mostly_ validated by its constructor, but the concrete request could still be invalid + if the resource entity is an enum entity and the id is not an instance of that enum. This cannot happen here because we draw candidate + resources from the entities, which we know are valid." + )] + let req = request.to_request(entity.uid().id().clone(), None).unwrap(); let authorizer = Authorizer::new(); - authorizer + let auth_response = authorizer .is_authorized( - &request.to_request(entity.uid().id().clone(), None).unwrap(), - policies, + &req, + &policies, entities, - ) - .decision - == Decision::Allow + ); + auth_response.decision() == Decision::Allow }) - .map(super::Entity::uid) + .map(Entity::uid) .collect_vec() .into_iter()), } @@ -744,45 +803,38 @@ impl PolicySet { schema: &Schema, ) -> Result, PermissionQueryError> { let partial_entities = PartialEntities::from_concrete(entities.clone(), schema)?; - let residuals = self.tpe(&request.0, &partial_entities, schema)?; - #[expect( - clippy::unwrap_used, - reason = "policy set construction should succeed because there shouldn't be any policy id conflicts" - )] - let policies = &Self::from_policies( - residuals - .0 - .residual_policies() - .map(|p| Policy::from_ast(p.clone().into())), - ) - .unwrap(); - #[expect( - clippy::unwrap_used, - reason = "request construction should succeed because each entity passes validation" - )] - match residuals.decision() { + let tpe_response = self.tpe(&request.0, &partial_entities, schema)?; + let policies = tpe_response.policy_set(); + match tpe_response.decision() { Some(Decision::Allow) => Ok(entities .iter() - .filter(|entity| entity.0.uid().entity_type() == &request.0 .0.get_principal_type()) - .map(super::Entity::uid) + .filter(|entity| entity.0.uid().entity_type() == &request.0.0.get_principal_type()) + .map(Entity::uid) .collect_vec() .into_iter()), Some(Decision::Deny) => Ok(vec![].into_iter()), None => Ok(entities .iter() - .filter(|entity| entity.0.uid().entity_type() == &request.0 .0.get_principal_type()) + .filter(|entity| entity.0.uid().entity_type() == &request.0.0.get_principal_type()) .filter(|entity| { + #[expect( + clippy::unwrap_used, reason = "`to_request` cannot panic because we do not pass a schema. However, the correctness of the authorization + decision depends on having valid a request and entities, but we do not do any validation here. Entities were already validated by + `PartialEntities::from_concrete`. The request was _mostly_ validated by its constructor, but the concrete request could still be invalid + if the principal entity is an enum entity and the id is not an instance of that enum. This cannot happen here because we draw candidate + principals from the entities, which we know are valid." + )] + let req = request.to_request(entity.uid().id().clone(), None).unwrap(); let authorizer = Authorizer::new(); - authorizer + let auth_response = authorizer .is_authorized( - &request.to_request(entity.uid().id().clone(), None).unwrap(), - policies, + &req, + &policies, entities, - ) - .decision - == Decision::Allow + ); + auth_response.decision() == Decision::Allow }) - .map(super::Entity::uid) + .map(Entity::uid) .collect_vec() .into_iter()), } @@ -1369,10 +1421,7 @@ unless .tpe(&request, &partial_entities, &schema) .expect("tpe should succeed"); - assert_eq!( - response.residual_policies().count(), - policies.num_of_policies() - ); + assert_eq!(response.policies().count(), policies.num_of_policies()); for p in response.residual_policies() { assert_matches!(p.action_constraint(), ActionConstraint::Any); assert_matches!(p.principal_constraint(), PrincipalConstraint::Any); @@ -1380,7 +1429,7 @@ unless } assert_eq!( response - .nontrivial_residual_policies() + .residual_policies() .next() .unwrap() .annotation("id") @@ -3012,7 +3061,7 @@ when { principal in resource.admins }; .tpe(&request, &partial_entities, &schema) .expect("tpe should succeed"); // There should be exactly one nontrivial residual - let residual_policies: Vec<_> = response.nontrivial_residual_policies().collect(); + let residual_policies: Vec<_> = response.residual_policies().collect(); assert_eq!( residual_policies.len(), 1, @@ -3191,7 +3240,7 @@ when { principal in resource.admins }; .to_pst() .unwrap(); - let residuals: Vec<_> = response.nontrivial_residual_policies().collect(); + let residuals: Vec<_> = response.residual_policies().collect(); assert_eq!(residuals[0].to_pst().unwrap().body(), expected.body()); assert_eq!(response.decision(), None); assert!(response.reason().is_none()); diff --git a/cedar-policy/src/proto/api.rs b/cedar-policy/src/proto/api.rs index 2b5476e28..261b18e4a 100644 --- a/cedar-policy/src/proto/api.rs +++ b/cedar-policy/src/proto/api.rs @@ -84,8 +84,7 @@ impl TryFrom for api::PolicySet { type Error = ProtobufConversionError; fn try_from(v: models::PolicySet) -> Result { let ast: cedar_policy_core::ast::PolicySet = v.try_into()?; - Self::from_ast(ast) - .map_err(|e| ProtobufConversionError::InvalidValue(format!("invalid policy set: {e}"))) + Ok(Self::from_ast(ast)) } } diff --git a/cedar-policy/src/proto/traits.rs b/cedar-policy/src/proto/traits.rs index 10afa011a..e27859c5d 100644 --- a/cedar-policy/src/proto/traits.rs +++ b/cedar-policy/src/proto/traits.rs @@ -19,9 +19,8 @@ use cedar_policy_core::{ entities::err::EntitiesError, validator::SchemaError, }; -use itertools::Either; -use crate::{api, PolicySetError}; +use crate::api; use super::ast::ProtobufConversionError; @@ -151,12 +150,9 @@ pub(crate) fn try_decode< } impl TryValidate for api::PolicySet { - type Err = Either; + type Err = PolicySetValidationError; fn try_validate(self) -> Result { - self.ast - .try_validate() - .map_err(Either::Right) - .and_then(|o| o.try_into().map_err(Either::Left)) + self.ast.try_validate().map(|o| o.into()) } }