From 6e677ed70a2511d3502d4faa4c6726b15f821336 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Wed, 10 Jun 2026 20:17:19 +0000 Subject: [PATCH 1/9] Add `TpeResponse::decision`, `TpeResponse::satisfied_permits`, and `TpeResponse::satisfied_forbids` Signed-off-by: John Kastner --- cedar-policy-core/src/tpe.rs | 12 ++--- cedar-policy-core/src/tpe/response.rs | 16 +++---- cedar-policy/CHANGELOG.md | 1 + cedar-policy/src/api/tpe.rs | 64 ++++++++++++++++++++++++--- 4 files changed, 72 insertions(+), 21 deletions(-) diff --git a/cedar-policy-core/src/tpe.rs b/cedar-policy-core/src/tpe.rs index ca5560aeb8..655e7babfd 100644 --- a/cedar-policy-core/src/tpe.rs +++ b/cedar-policy-core/src/tpe.rs @@ -457,36 +457,36 @@ when { principal in resource.editors }; .static_policies() .find(|p| matches!(p.annotation(&id), Some(Annotation {val, ..}) if val == "3")) .unwrap(); - let false_permits: HashSet = residuals + 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 = residuals + let false_forbids: HashSet<&PolicyID> = residuals .false_forbids() .map(|p| p.get_policy_id()) .collect(); assert!(false_forbids.is_empty()); - let true_permits: HashSet = residuals + let true_permits: HashSet<&PolicyID> = residuals .satisfied_permits() .map(|p| p.get_policy_id()) .collect(); assert!(true_permits.is_empty()); - let true_forbids: HashSet = residuals + 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 = residuals + 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 = residuals + let non_trivial_forbids: HashSet<&PolicyID> = residuals .residual_forbids() .map(|p| p.get_policy_id()) .collect(); diff --git a/cedar-policy-core/src/tpe/response.rs b/cedar-policy-core/src/tpe/response.rs index 253feb79b8..f844078018 100644 --- a/cedar-policy-core/src/tpe/response.rs +++ b/cedar-policy-core/src/tpe/response.rs @@ -59,8 +59,8 @@ impl ResidualPolicy { } /// Get the [`PolicyID`] - pub fn get_policy_id(&self) -> PolicyID { - self.policy.id().clone() + pub fn get_policy_id(&self) -> &PolicyID { + self.policy.id() } /// All literal uids referenced by this residual @@ -130,20 +130,20 @@ impl<'a> Response<'a> { match rp.get_effect() { Effect::Forbid => { if r.is_true() { - satisfied_forbids.insert(id); + satisfied_forbids.insert(id.clone()); } else if r.is_false() || r.is_error() { - false_forbids.insert(id); + false_forbids.insert(id.clone()); } else { - residual_forbids.insert(id); + residual_forbids.insert(id.clone()); } } Effect::Permit => { if r.is_true() { - satisfied_permits.insert(id); + satisfied_permits.insert(id.clone()); } else if r.is_false() || r.is_error() { - false_permits.insert(id); + false_permits.insert(id.clone()); } else { - residual_permits.insert(id); + residual_permits.insert(id.clone()); } } } diff --git a/cedar-policy/CHANGELOG.md b/cedar-policy/CHANGELOG.md index 9cd0eeccd6..2d3cc51457 100644 --- a/cedar-policy/CHANGELOG.md +++ b/cedar-policy/CHANGELOG.md @@ -14,6 +14,7 @@ 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 the determining policies on a TPE Response (`TpeResponse::decision`, `TpeResponse::satisfied_permits`, and `TpeResponse::satisfied_forbids`) ### Changed diff --git a/cedar-policy/src/api/tpe.rs b/cedar-policy/src/api/tpe.rs index e3e4ee4010..aaff90edc6 100644 --- a/cedar-policy/src/api/tpe.rs +++ b/cedar-policy/src/api/tpe.rs @@ -32,8 +32,9 @@ use smol_str::SmolStr; use crate::{ api, tpe_err, Authorizer, Context, Entities, Entity, EntityId, EntityTypeName, EntityUid, - PartialEntityError, PartialRequestCreationError, PermissionQueryError, Policy, PolicySet, - Request, RequestValidationError, RestrictedExpression, Schema, TpeReauthorizationError, + PartialEntityError, PartialRequestCreationError, PermissionQueryError, Policy, PolicyId, + PolicySet, Request, RequestValidationError, RestrictedExpression, Schema, + TpeReauthorizationError, }; /// A partial [`EntityUid`]. @@ -384,6 +385,44 @@ impl TpeResponse<'_> { self.0.decision() } + /// Get the determing policies for the partial authorization decision. + /// These are a subset of the determining polices for any subsequent + /// concrete reauthorization. + /// + /// When [`TpeResponse::decision`] returns a concrete allow or deny, the + /// determining policies are the satisfied permits or forbids respectivly. + /// If TPE did not reach a concrete decision, then this will always return + /// an empty set. + pub fn reason(&self) -> HashSet<&PolicyId> { + match self.0.decision() { + None => HashSet::new(), + Some(Decision::Allow) => self.satisfied_permits().collect(), + Some(Decision::Deny) => self.satisfied_forbids().collect(), + } + } + + /// Get the permit policies that are concretly satisfied by the partial request. + /// + /// If the eventual concrete authorization decision is allow, then these + /// will be a subset of the determining policies. However, they may still be + /// overriden by a non-trivial residual forbid policy. + pub fn satisfied_permits(&self) -> impl Iterator { + self.0 + .satisfied_permits() + .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) + } + + /// Get the forbid policies that are concretly satisfied by the partial request. + /// + /// Presence of any satisfied forbids guarenteees that they are exactly the + /// policies returned by [`TpeResponse::reason`] and that [`TpeResponse::decision`] + /// must return `Deny`. + pub fn satisfied_forbids(&self) -> impl Iterator { + self.0 + .satisfied_forbids() + .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) + } + /// Perform reauthorization pub fn reauthorize( &self, @@ -818,7 +857,7 @@ mod tpe_tests { } mod streaming_service { - use std::{collections::BTreeMap, str::FromStr}; + use std::{collections::{BTreeMap, HashSet}, str::FromStr}; use cedar_policy_core::{authorizer::Decision, tpe::err::EntitiesError}; use cool_asserts::assert_matches; @@ -1259,6 +1298,7 @@ unless ); assert_eq!(response.decision(), None); + assert_eq!(response.reason(), HashSet::new()); let request = Request::new( EntityUid::from_type_name_and_id( @@ -2115,10 +2155,9 @@ when { principal in resource.admins }; use itertools::Itertools; use crate::{ - Context, Entities, PartialEntities, PartialEntityUid, PartialRequest, PolicySet, - PrincipalQueryRequest, ResourceQueryRequest, Schema, + Context, Entities, PartialEntities, PartialEntityUid, PartialRequest, PolicyId, PolicySet, PrincipalQueryRequest, ResourceQueryRequest, Schema }; - use std::{i64, str::FromStr}; + use std::{collections::HashSet, i64, str::FromStr}; fn schema() -> Schema { Schema::from_str("entity P, R; action A appliesTo { principal: P, resource: R };") @@ -2153,6 +2192,7 @@ when { principal in resource.admins }; .tpe(&req, &partial_entities, &schema) .unwrap(); assert_eq!(response.decision(), Some(Decision::Allow)); + assert_eq!(response.reason(), HashSet::from([&PolicyId::new("policy0")])); } #[test] @@ -2214,6 +2254,7 @@ when { principal in resource.admins }; .tpe(&req, &partial_entities, &schema) .unwrap(); assert_eq!(response.decision(), Some(Decision::Deny)); + assert_eq!(response.reason(), HashSet::from([&PolicyId::new("policy0")])); } #[test] @@ -2278,6 +2319,7 @@ when { principal in resource.admins }; .tpe(&req, &partial_entities, &schema) .unwrap(); assert_eq!(response.decision(), Some(Decision::Deny)); + assert_eq!(response.reason(), HashSet::new()); } #[test] @@ -2345,6 +2387,7 @@ when { principal in resource.admins }; .tpe(&req, &partial_entities, &schema) .unwrap(); assert_eq!(response.decision(), Some(Decision::Deny)); + assert_eq!(response.reason(), HashSet::new()); } #[test] @@ -2815,7 +2858,10 @@ when { principal in resource.admins }; } mod template_links { - use std::{collections::HashMap, str::FromStr}; + use std::{ + collections::{HashMap, HashSet}, + str::FromStr, + }; use crate::{ pst, Decision, EntityUid, PartialEntities, PartialEntityUid, PartialRequest, Policy, @@ -2877,6 +2923,7 @@ when { principal in resource.admins }; let response = policies.tpe(&request, &es, &schema).unwrap(); assert_eq!(response.decision(), Some(Decision::Allow)); + assert_eq!(response.reason(), HashSet::from([&PolicyId::new("l")])); } #[test] @@ -2889,6 +2936,7 @@ when { principal in resource.admins }; let response = policies.tpe(&request, &es, &schema).unwrap(); assert_eq!(response.decision(), Some(Decision::Deny)); + assert_eq!(response.reason(), HashSet::new()); } #[test] @@ -2911,6 +2959,7 @@ when { principal in resource.admins }; let response = policies.tpe(&request, &es, &schema).unwrap(); assert_eq!(response.decision(), Some(Decision::Deny)); + assert_eq!(response.reason(), HashSet::new()); } #[test] @@ -2943,6 +2992,7 @@ when { principal in resource.admins }; let residuals: Vec<_> = response.nontrivial_residual_policies().collect(); assert_eq!(residuals[0].to_pst().unwrap().body(), expected.body()); assert_eq!(response.decision(), None); + assert_eq!(response.reason(), HashSet::new()); assert_eq!(residuals.len(), 1); } } From 8197a5a934ff5e963642c3f47eaac80c1ed6f873 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Wed, 10 Jun 2026 20:57:02 +0000 Subject: [PATCH 2/9] fmt Signed-off-by: John Kastner --- cedar-policy/src/api/tpe.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/cedar-policy/src/api/tpe.rs b/cedar-policy/src/api/tpe.rs index aaff90edc6..ff48c953e0 100644 --- a/cedar-policy/src/api/tpe.rs +++ b/cedar-policy/src/api/tpe.rs @@ -857,7 +857,10 @@ mod tpe_tests { } mod streaming_service { - use std::{collections::{BTreeMap, HashSet}, str::FromStr}; + use std::{ + collections::{BTreeMap, HashSet}, + str::FromStr, + }; use cedar_policy_core::{authorizer::Decision, tpe::err::EntitiesError}; use cool_asserts::assert_matches; @@ -2155,7 +2158,8 @@ when { principal in resource.admins }; use itertools::Itertools; use crate::{ - Context, Entities, PartialEntities, PartialEntityUid, PartialRequest, PolicyId, PolicySet, PrincipalQueryRequest, ResourceQueryRequest, Schema + Context, Entities, PartialEntities, PartialEntityUid, PartialRequest, PolicyId, + PolicySet, PrincipalQueryRequest, ResourceQueryRequest, Schema, }; use std::{collections::HashSet, i64, str::FromStr}; @@ -2192,7 +2196,10 @@ when { principal in resource.admins }; .tpe(&req, &partial_entities, &schema) .unwrap(); assert_eq!(response.decision(), Some(Decision::Allow)); - assert_eq!(response.reason(), HashSet::from([&PolicyId::new("policy0")])); + assert_eq!( + response.reason(), + HashSet::from([&PolicyId::new("policy0")]) + ); } #[test] @@ -2254,7 +2261,10 @@ when { principal in resource.admins }; .tpe(&req, &partial_entities, &schema) .unwrap(); assert_eq!(response.decision(), Some(Decision::Deny)); - assert_eq!(response.reason(), HashSet::from([&PolicyId::new("policy0")])); + assert_eq!( + response.reason(), + HashSet::from([&PolicyId::new("policy0")]) + ); } #[test] From 02763830877357a7d65ae52164a965c0f49cdbad Mon Sep 17 00:00:00 2001 From: John Kastner <130772734+john-h-kastner-aws@users.noreply.github.com> Date: Wed, 10 Jun 2026 17:23:27 -0400 Subject: [PATCH 3/9] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- cedar-policy/CHANGELOG.md | 2 +- cedar-policy/src/api/tpe.rs | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cedar-policy/CHANGELOG.md b/cedar-policy/CHANGELOG.md index 2d3cc51457..366b4d0ea0 100644 --- a/cedar-policy/CHANGELOG.md +++ b/cedar-policy/CHANGELOG.md @@ -14,7 +14,7 @@ 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 the determining policies on a TPE Response (`TpeResponse::decision`, `TpeResponse::satisfied_permits`, and `TpeResponse::satisfied_forbids`) +- Functions for inspecting the determining policies on a TPE response (`TpeResponse::reason`, `TpeResponse::satisfied_permits`, and `TpeResponse::satisfied_forbids`). ### Changed diff --git a/cedar-policy/src/api/tpe.rs b/cedar-policy/src/api/tpe.rs index ff48c953e0..8e6740754d 100644 --- a/cedar-policy/src/api/tpe.rs +++ b/cedar-policy/src/api/tpe.rs @@ -385,12 +385,12 @@ impl TpeResponse<'_> { self.0.decision() } - /// Get the determing policies for the partial authorization decision. - /// These are a subset of the determining polices for any subsequent + /// Get the determining policies for the partial authorization decision. + /// These are a subset of the determining policies for any subsequent /// concrete reauthorization. /// /// When [`TpeResponse::decision`] returns a concrete allow or deny, the - /// determining policies are the satisfied permits or forbids respectivly. + /// determining policies are the satisfied permits or forbids respectively. /// If TPE did not reach a concrete decision, then this will always return /// an empty set. pub fn reason(&self) -> HashSet<&PolicyId> { @@ -401,20 +401,20 @@ impl TpeResponse<'_> { } } - /// Get the permit policies that are concretly satisfied by the partial request. + /// Get the permit policies that are concretely satisfied by the partial request. /// /// If the eventual concrete authorization decision is allow, then these /// will be a subset of the determining policies. However, they may still be - /// overriden by a non-trivial residual forbid policy. + /// overridden by a non-trivial residual forbid policy. pub fn satisfied_permits(&self) -> impl Iterator { self.0 .satisfied_permits() .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) } - /// Get the forbid policies that are concretly satisfied by the partial request. + /// Get the forbid policies that are concretely satisfied by the partial request. /// - /// Presence of any satisfied forbids guarenteees that they are exactly the + /// Presence of any satisfied forbids guarantees that they are exactly the /// policies returned by [`TpeResponse::reason`] and that [`TpeResponse::decision`] /// must return `Deny`. pub fn satisfied_forbids(&self) -> impl Iterator { From 2da907c5fdb95f56f7e3843be4202a12e1da48fa Mon Sep 17 00:00:00 2001 From: John Kastner Date: Thu, 11 Jun 2026 18:07:57 +0000 Subject: [PATCH 4/9] Expand doc comments Signed-off-by: John Kastner --- cedar-policy-core/src/tpe/response.rs | 32 +++++---- cedar-policy/src/api/tpe.rs | 97 +++++++++++++++++---------- 2 files changed, 82 insertions(+), 47 deletions(-) diff --git a/cedar-policy-core/src/tpe/response.rs b/cedar-policy-core/src/tpe/response.rs index f844078018..a3ba4c071d 100644 --- a/cedar-policy-core/src/tpe/response.rs +++ b/cedar-policy-core/src/tpe/response.rs @@ -88,13 +88,13 @@ pub struct Response<'a> { decision: Option, residuals: HashMap, // All of the [`Effect::Permit`] policies that were satisfied - satisfied_permits: HashSet, + true_permits: HashSet, // All of the [`Effect::Permit`] policies that were not satisfied false_permits: HashSet, // All of the [`Effect::Permit`] policies that evaluated to a residual residual_permits: HashSet, // All of the [`Effect::Forbid`] policies that were satisfied - satisfied_forbids: HashSet, + true_forbids: HashSet, // All of the [`Effect::Forbid`] policies that were not satisfied false_forbids: HashSet, // All of the [`Effect::Forbid`] policies that evaluated to a residual @@ -117,10 +117,10 @@ impl<'a> Response<'a> { schema: &'a ValidatorSchema, ) -> Self { let mut residual_map = HashMap::new(); - let mut satisfied_permits = HashSet::new(); + let mut true_permits = HashSet::new(); let mut false_permits = HashSet::new(); let mut residual_permits = HashSet::new(); - let mut satisfied_forbids = HashSet::new(); + let mut true_forbids = HashSet::new(); let mut false_forbids = HashSet::new(); let mut residual_forbids = HashSet::new(); for rp in residuals { @@ -130,7 +130,7 @@ impl<'a> Response<'a> { match rp.get_effect() { Effect::Forbid => { if r.is_true() { - satisfied_forbids.insert(id.clone()); + true_forbids.insert(id.clone()); } else if r.is_false() || r.is_error() { false_forbids.insert(id.clone()); } else { @@ -139,7 +139,7 @@ impl<'a> Response<'a> { } Effect::Permit => { if r.is_true() { - satisfied_permits.insert(id.clone()); + true_permits.insert(id.clone()); } else if r.is_false() || r.is_error() { false_permits.insert(id.clone()); } else { @@ -150,8 +150,8 @@ impl<'a> Response<'a> { } let decision = match ( - !satisfied_forbids.is_empty(), - !satisfied_permits.is_empty(), + !true_forbids.is_empty(), + !true_permits.is_empty(), !residual_permits.is_empty(), !residual_forbids.is_empty(), ) { @@ -170,10 +170,10 @@ impl<'a> Response<'a> { Self { decision, residuals: residual_map, - satisfied_permits, + true_permits, false_permits, residual_permits, - satisfied_forbids, + true_forbids, false_forbids, residual_forbids, request, @@ -188,7 +188,7 @@ impl<'a> Response<'a> { clippy::unwrap_used, reason = "we know that the policy ids are in the residuals map" )] - self.satisfied_permits + self.true_permits .iter() .map(|id| self.residuals.get(id).unwrap()) } @@ -199,7 +199,7 @@ impl<'a> Response<'a> { clippy::unwrap_used, reason = "we know that the policy ids are in the residuals map" )] - self.satisfied_forbids + self.true_forbids .iter() .map(|id| self.residuals.get(id).unwrap()) } @@ -258,6 +258,14 @@ impl<'a> Response<'a> { self.decision } + /// Get the determining policies for the authorization decision + pub fn reason(&self) -> Option> { + match self.decision? { + Decision::Allow => Some(self.true_permits.iter()), + Decision::Deny => Some(self.true_forbids.iter()), + } + } + /// Perform reauthorization pub fn reauthorize( &self, diff --git a/cedar-policy/src/api/tpe.rs b/cedar-policy/src/api/tpe.rs index 8e6740754d..e18ed2b1ad 100644 --- a/cedar-policy/src/api/tpe.rs +++ b/cedar-policy/src/api/tpe.rs @@ -390,23 +390,33 @@ impl TpeResponse<'_> { /// concrete reauthorization. /// /// When [`TpeResponse::decision`] returns a concrete allow or deny, the - /// determining policies are the satisfied permits or forbids respectively. - /// If TPE did not reach a concrete decision, then this will always return - /// an empty set. - pub fn reason(&self) -> HashSet<&PolicyId> { - match self.0.decision() { - None => HashSet::new(), - Some(Decision::Allow) => self.satisfied_permits().collect(), - Some(Decision::Deny) => self.satisfied_forbids().collect(), - } + /// determining policies returned by this function are the satisfied permits + /// or forbids respectively. + /// + /// If partial authorization does not reach a decision, then this function + /// returns `None`. It's reasonable to treat this response as "no known + /// determining policies", in which case you can call this function as + /// `response.reason().into_iter().flatten()`. + pub fn reason(&self) -> Option> { + Some(self.0.reason()?.map(PolicyId::ref_cast)) } /// Get the permit policies that are concretely satisfied by the partial request. /// - /// If the eventual concrete authorization decision is allow, then these - /// will be a subset of the determining policies. However, they may still be - /// overridden by a non-trivial residual forbid policy. - pub fn satisfied_permits(&self) -> impl Iterator { + /// To properly interpret the ids returned from this function you need to + /// consider them in the context of [`TpeResponse::decision`]: + /// * For a concrete `Allow` decision, these are a subset of the concrete + /// determining policies and are exactly the policies returned by + /// [`TpeResponse::reason`]. + /// * For a concrete `Deny` decision, these are not determining policies. The + /// iterator may be empty if no permits were satisfied, or it may contain + /// satisfied permits which have been overridden by at least one satisfied + /// forbid policy. + /// * For an unknown decision, these will be a subset of the determining + /// policies _if_ the eventual concrete authorization decision is `Allow`, + /// but they may still be overridden by any non-trivial residual forbid + /// policy. + pub fn true_permits(&self) -> impl Iterator { self.0 .satisfied_permits() .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) @@ -416,8 +426,8 @@ impl TpeResponse<'_> { /// /// Presence of any satisfied forbids guarantees that they are exactly the /// policies returned by [`TpeResponse::reason`] and that [`TpeResponse::decision`] - /// must return `Deny`. - pub fn satisfied_forbids(&self) -> impl Iterator { + /// must return a concrete `Deny`. + pub fn true_forbids(&self) -> impl Iterator { self.0 .satisfied_forbids() .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) @@ -857,10 +867,7 @@ mod tpe_tests { } mod streaming_service { - use std::{ - collections::{BTreeMap, HashSet}, - str::FromStr, - }; + use std::{collections::BTreeMap, str::FromStr}; use cedar_policy_core::{authorizer::Decision, tpe::err::EntitiesError}; use cool_asserts::assert_matches; @@ -1301,7 +1308,7 @@ unless ); assert_eq!(response.decision(), None); - assert_eq!(response.reason(), HashSet::new()); + assert!(response.reason().is_none()); let request = Request::new( EntityUid::from_type_name_and_id( @@ -2161,7 +2168,7 @@ when { principal in resource.admins }; Context, Entities, PartialEntities, PartialEntityUid, PartialRequest, PolicyId, PolicySet, PrincipalQueryRequest, ResourceQueryRequest, Schema, }; - use std::{collections::HashSet, i64, str::FromStr}; + use std::{i64, str::FromStr}; fn schema() -> Schema { Schema::from_str("entity P, R; action A appliesTo { principal: P, resource: R };") @@ -2197,8 +2204,8 @@ when { principal in resource.admins }; .unwrap(); assert_eq!(response.decision(), Some(Decision::Allow)); assert_eq!( - response.reason(), - HashSet::from([&PolicyId::new("policy0")]) + response.reason().unwrap().collect::>(), + vec![&PolicyId::new("policy0")] ); } @@ -2262,8 +2269,12 @@ when { principal in resource.admins }; .unwrap(); assert_eq!(response.decision(), Some(Decision::Deny)); assert_eq!( - response.reason(), - HashSet::from([&PolicyId::new("policy0")]) + response.reason().unwrap().collect::>(), + vec![&PolicyId::new("policy0")] + ); + assert_eq!( + response.true_forbids().collect::>(), + vec![&PolicyId::new("policy0")] ); } @@ -2329,7 +2340,10 @@ when { principal in resource.admins }; .tpe(&req, &partial_entities, &schema) .unwrap(); assert_eq!(response.decision(), Some(Decision::Deny)); - assert_eq!(response.reason(), HashSet::new()); + assert_eq!( + response.reason().unwrap().collect::>(), + Vec::<&PolicyId>::new() + ); } #[test] @@ -2397,7 +2411,10 @@ when { principal in resource.admins }; .tpe(&req, &partial_entities, &schema) .unwrap(); assert_eq!(response.decision(), Some(Decision::Deny)); - assert_eq!(response.reason(), HashSet::new()); + assert_eq!( + response.reason().unwrap().collect::>(), + Vec::<&PolicyId>::new() + ); } #[test] @@ -2868,10 +2885,7 @@ when { principal in resource.admins }; } mod template_links { - use std::{ - collections::{HashMap, HashSet}, - str::FromStr, - }; + use std::{collections::HashMap, str::FromStr}; use crate::{ pst, Decision, EntityUid, PartialEntities, PartialEntityUid, PartialRequest, Policy, @@ -2933,7 +2947,14 @@ when { principal in resource.admins }; let response = policies.tpe(&request, &es, &schema).unwrap(); assert_eq!(response.decision(), Some(Decision::Allow)); - assert_eq!(response.reason(), HashSet::from([&PolicyId::new("l")])); + assert_eq!( + response.reason().unwrap().collect::>(), + vec![&PolicyId::new("l")] + ); + assert_eq!( + response.true_permits().collect::>(), + vec![&PolicyId::new("l")] + ); } #[test] @@ -2946,7 +2967,10 @@ when { principal in resource.admins }; let response = policies.tpe(&request, &es, &schema).unwrap(); assert_eq!(response.decision(), Some(Decision::Deny)); - assert_eq!(response.reason(), HashSet::new()); + assert_eq!( + response.reason().unwrap().collect::>(), + Vec::<&PolicyId>::new() + ); } #[test] @@ -2969,7 +2993,10 @@ when { principal in resource.admins }; let response = policies.tpe(&request, &es, &schema).unwrap(); assert_eq!(response.decision(), Some(Decision::Deny)); - assert_eq!(response.reason(), HashSet::new()); + assert_eq!( + response.reason().unwrap().collect::>(), + Vec::<&PolicyId>::new() + ); } #[test] @@ -3002,7 +3029,7 @@ when { principal in resource.admins }; let residuals: Vec<_> = response.nontrivial_residual_policies().collect(); assert_eq!(residuals[0].to_pst().unwrap().body(), expected.body()); assert_eq!(response.decision(), None); - assert_eq!(response.reason(), HashSet::new()); + assert!(response.reason().is_none()); assert_eq!(residuals.len(), 1); } } From a1b7f39e3c1e9caacf085ed267b810f6dfc69a23 Mon Sep 17 00:00:00 2001 From: John Kastner <130772734+john-h-kastner-aws@users.noreply.github.com> Date: Mon, 15 Jun 2026 10:04:49 -0400 Subject: [PATCH 5/9] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- cedar-policy/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cedar-policy/CHANGELOG.md b/cedar-policy/CHANGELOG.md index 366b4d0ea0..536c3eaf29 100644 --- a/cedar-policy/CHANGELOG.md +++ b/cedar-policy/CHANGELOG.md @@ -14,7 +14,7 @@ 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 the determining policies on a TPE response (`TpeResponse::reason`, `TpeResponse::satisfied_permits`, and `TpeResponse::satisfied_forbids`). +- Functions for inspecting the determining policies on a TPE response (`TpeResponse::reason`, `TpeResponse::true_permits`, and `TpeResponse::true_forbids`). ### Changed From 85ce1f9b0a665f6a99ab4d1904c89e5efece5db2 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 16 Jun 2026 21:43:40 +0000 Subject: [PATCH 6/9] Expand API furhter Signed-off-by: John Kastner --- cedar-policy-core/src/tpe/response.rs | 46 +++++++-- cedar-policy/src/api/tpe.rs | 131 ++++++++++++++++++++++++++ 2 files changed, 171 insertions(+), 6 deletions(-) diff --git a/cedar-policy-core/src/tpe/response.rs b/cedar-policy-core/src/tpe/response.rs index a3ba4c071d..b78df1900c 100644 --- a/cedar-policy-core/src/tpe/response.rs +++ b/cedar-policy-core/src/tpe/response.rs @@ -87,16 +87,20 @@ impl From for Policy { pub struct Response<'a> { decision: Option, residuals: HashMap, - // All of the [`Effect::Permit`] policies that were satisfied + // All of the [`Effect::Permit`] policies that were true true_permits: HashSet, - // All of the [`Effect::Permit`] policies that were not satisfied + // All of the [`Effect::Permit`] policies that were false false_permits: HashSet, + // All of the [`Effect::Permit`] policies that errored + error_permits: HashSet, // All of the [`Effect::Permit`] policies that evaluated to a residual residual_permits: HashSet, - // All of the [`Effect::Forbid`] policies that were satisfied + // All of the [`Effect::Forbid`] policies that were true true_forbids: HashSet, - // All of the [`Effect::Forbid`] policies that were not satisfied + // All of the [`Effect::Forbid`] policies that were false false_forbids: HashSet, + // All of the [`Effect::Forbid`] policies that errored + error_forbids: HashSet, // All of the [`Effect::Forbid`] policies that evaluated to a residual residual_forbids: HashSet, // request used for this partial evaluation @@ -119,9 +123,11 @@ impl<'a> Response<'a> { let mut residual_map = HashMap::new(); let mut true_permits = HashSet::new(); let mut false_permits = HashSet::new(); + let mut error_permits = HashSet::new(); let mut residual_permits = HashSet::new(); let mut true_forbids = HashSet::new(); let mut false_forbids = HashSet::new(); + let mut error_forbids = HashSet::new(); let mut residual_forbids = HashSet::new(); for rp in residuals { let r = rp.get_residual(); @@ -131,8 +137,10 @@ impl<'a> Response<'a> { Effect::Forbid => { if r.is_true() { true_forbids.insert(id.clone()); - } else if r.is_false() || r.is_error() { + } else if r.is_false() { false_forbids.insert(id.clone()); + } else if r.is_error() { + error_forbids.insert(id.clone()); } else { residual_forbids.insert(id.clone()); } @@ -140,8 +148,10 @@ impl<'a> Response<'a> { Effect::Permit => { if r.is_true() { true_permits.insert(id.clone()); - } else if r.is_false() || r.is_error() { + } else if r.is_false() { false_permits.insert(id.clone()); + } else if r.is_error() { + error_permits.insert(id.clone()); } else { residual_permits.insert(id.clone()); } @@ -172,9 +182,11 @@ impl<'a> Response<'a> { residuals: residual_map, true_permits, false_permits, + error_permits, residual_permits, true_forbids, false_forbids, + error_forbids, residual_forbids, request, entities, @@ -215,6 +227,17 @@ impl<'a> Response<'a> { .map(|id| self.residuals.get(id).unwrap()) } + /// Get trivially erroring permit residual policies + pub fn error_permits(&self) -> impl Iterator { + #[expect( + clippy::unwrap_used, + reason = "we know that the policy ids are in the residuals map" + )] + self.error_permits + .iter() + .map(|id| self.residuals.get(id).unwrap()) + } + /// Get trivially false forbid residual policies pub fn false_forbids(&self) -> impl Iterator { #[expect( @@ -226,6 +249,17 @@ impl<'a> Response<'a> { .map(|id| self.residuals.get(id).unwrap()) } + /// Get trivially erroring forbid residual policies + pub fn error_forbids(&self) -> impl Iterator { + #[expect( + clippy::unwrap_used, + reason = "we know that the policy ids are in the residuals map" + )] + self.error_forbids + .iter() + .map(|id| self.residuals.get(id).unwrap()) + } + /// Get non-trivial permit residual policies pub fn residual_permits(&self) -> impl Iterator { #[expect( diff --git a/cedar-policy/src/api/tpe.rs b/cedar-policy/src/api/tpe.rs index e18ed2b1ad..d9186133bb 100644 --- a/cedar-policy/src/api/tpe.rs +++ b/cedar-policy/src/api/tpe.rs @@ -401,6 +401,21 @@ impl TpeResponse<'_> { Some(self.0.reason()?.map(PolicyId::ref_cast)) } + /// Get the permit 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`]. + /// + /// These policies could be determining policies _if_ the eventual + /// concrete authorization decision is `Allow` _and_ they are satisfied by + /// the concrete request. If the [`TpeResponse::decision`] is `Deny`, then + /// they cannot be determining. + pub fn residual_permits(&self) -> impl Iterator { + self.0 + .residual_permits() + .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) + } + /// Get the permit policies that are concretely satisfied by the partial request. /// /// To properly interpret the ids returned from this function you need to @@ -422,6 +437,43 @@ impl TpeResponse<'_> { .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) } + /// Get the permit policies that are concretely not satisfied by the partial request. + /// + /// These policies evaluate to `false`, so they have no impact on the + /// partial authorization decision or on any subsequent concrete decision + /// after reauthorization. + pub fn false_permits(&self) -> impl Iterator { + self.0 + .false_permits() + .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) + } + + /// Get the permit policies that encountered concrete errors for the partial request. + /// + /// These policies errored, so they have no impact on the partial + /// authorization decision. Erroring policies are not generally expected + /// since partial evaluation works only on _validated_ policies, but it is still + /// possible to encounter errors, e.g., on integer overflow. + pub fn error_permits(&self) -> impl Iterator { + self.0 + .error_permits() + .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) + } + + /// 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`]. + /// + /// 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 + /// policies do not apply, so they might still override any satisfied permit policies. + pub fn residual_forbids(&self) -> impl Iterator { + self.0 + .residual_forbids() + .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) + } + /// Get the forbid policies that are concretely satisfied by the partial request. /// /// Presence of any satisfied forbids guarantees that they are exactly the @@ -433,6 +485,29 @@ impl TpeResponse<'_> { .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) } + /// Get the forbid policies that are concretely not satisfied by the partial request. + /// + /// These policies evaluate to `false`, so they have no impact on the + /// partial authorization decision or on any subsequent concrete decision + /// after reauthorization. + pub fn false_forbids(&self) -> impl Iterator { + self.0 + .false_forbids() + .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) + } + + /// Get the forbid policies that encountered concrete errors for the partial request. + /// + /// These policies errored, so they have no impact on the partial + /// authorization decision. Erroring policies are not generally expected + /// since partial evaluation works only on _validated_ policies, but it is still + /// possible to encounter errors, e.g., on integer overflow. + pub fn error_forbids(&self) -> impl Iterator { + self.0 + .error_forbids() + .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) + } + /// Perform reauthorization pub fn reauthorize( &self, @@ -2460,6 +2535,62 @@ when { principal in resource.admins }; } } + mod response_iterators { + use std::{i64, str::FromStr}; + + use cedar_policy_core::authorizer::Decision; + + use crate::{ + PartialEntities, PartialEntityUid, PartialRequest, PolicyId, PolicySet, Schema, + }; + + #[test] + fn all_policy_categories() { + let schema = Schema::from_str( + "entity P, R; action A appliesTo { principal: P, resource: R, context: { flag: Bool } };", + ) + .unwrap(); + let req = PartialRequest::new( + PartialEntityUid::new("P".parse().unwrap(), None), + r#"Action::"A""#.parse().unwrap(), + PartialEntityUid::new("R".parse().unwrap(), None), + None, + &schema, + ) + .unwrap(); + + let policies = PolicySet::from_str(&format!( + r#" + permit(principal, action, resource); + permit(principal, action, resource) when {{ false }}; + permit(principal, action, resource) when {{ ({} + 1) == 0 || true }}; + permit(principal, action, resource) when {{ context.flag }}; + forbid(principal, action, resource); + forbid(principal, action, resource) when {{ false }}; + forbid(principal, action, resource) when {{ ({} + 1) == 0 || true }}; + forbid(principal, action, resource) when {{ context.flag }}; + "#, + i64::MAX, i64::MAX + )) + .unwrap(); + + let entities = PartialEntities::empty(); + let response = policies.tpe(&req, &entities, &schema).unwrap(); + + assert_eq!(response.decision(), Some(Decision::Deny)); + assert_eq!(response.reason().unwrap().collect::>(), vec![&PolicyId::new("policy4")]); + + assert_eq!(response.true_permits().collect::>(), vec![&PolicyId::new("policy0")]); + assert_eq!(response.false_permits().collect::>(), vec![&PolicyId::new("policy1")]); + assert_eq!(response.error_permits().collect::>(), vec![&PolicyId::new("policy2")]); + assert_eq!(response.residual_permits().collect::>(), vec![&PolicyId::new("policy3")]); + assert_eq!(response.true_forbids().collect::>(), vec![&PolicyId::new("policy4")]); + assert_eq!(response.false_forbids().collect::>(), vec![&PolicyId::new("policy5")]); + assert_eq!(response.error_forbids().collect::>(), vec![&PolicyId::new("policy6")]); + assert_eq!(response.residual_forbids().collect::>(), vec![&PolicyId::new("policy7")]); + } + } + mod query_action { use cedar_policy_core::authorizer::Decision; From a1b0e6abf6be1eeb59e5b875ab2c5c19da628797 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 16 Jun 2026 21:52:47 +0000 Subject: [PATCH 7/9] tweak Signed-off-by: John Kastner --- cedar-policy/src/api/tpe.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/cedar-policy/src/api/tpe.rs b/cedar-policy/src/api/tpe.rs index d9186133bb..f6b6e53708 100644 --- a/cedar-policy/src/api/tpe.rs +++ b/cedar-policy/src/api/tpe.rs @@ -386,8 +386,8 @@ impl TpeResponse<'_> { } /// Get the determining policies for the partial authorization decision. - /// These are a subset of the determining policies for any subsequent - /// concrete reauthorization. + /// These are a subset of the determining policies in the response returned + /// after calling [`TpeResponse::reauthorize`] with a concrete request and entities. /// /// When [`TpeResponse::decision`] returns a concrete allow or deny, the /// determining policies returned by this function are the satisfied permits @@ -508,7 +508,13 @@ impl TpeResponse<'_> { .map(|rp| PolicyId::ref_cast(rp.get_policy_id())) } - /// Perform reauthorization + /// Perform reauthorization, taking the residual policies and further + /// evaluating them with a concrete request and entities. + /// + /// If [`TpeResponse::decision`] returns a decision, then reauthorization + /// will always reach the same decision. If it does not, then this function + /// allows you to provide any data omitted from the partial request in order + /// to reach a concrete decision. pub fn reauthorize( &self, request: &Request, From feccd1ace81af36a2e000e327924136d32bb1047 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 16 Jun 2026 21:54:20 +0000 Subject: [PATCH 8/9] fmt Signed-off-by: John Kastner --- cedar-policy/src/api/tpe.rs | 50 +++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/cedar-policy/src/api/tpe.rs b/cedar-policy/src/api/tpe.rs index f6b6e53708..4ca1eaa940 100644 --- a/cedar-policy/src/api/tpe.rs +++ b/cedar-policy/src/api/tpe.rs @@ -2576,7 +2576,8 @@ when { principal in resource.admins }; forbid(principal, action, resource) when {{ ({} + 1) == 0 || true }}; forbid(principal, action, resource) when {{ context.flag }}; "#, - i64::MAX, i64::MAX + i64::MAX, + i64::MAX )) .unwrap(); @@ -2584,16 +2585,43 @@ when { principal in resource.admins }; let response = policies.tpe(&req, &entities, &schema).unwrap(); assert_eq!(response.decision(), Some(Decision::Deny)); - assert_eq!(response.reason().unwrap().collect::>(), vec![&PolicyId::new("policy4")]); - - assert_eq!(response.true_permits().collect::>(), vec![&PolicyId::new("policy0")]); - assert_eq!(response.false_permits().collect::>(), vec![&PolicyId::new("policy1")]); - assert_eq!(response.error_permits().collect::>(), vec![&PolicyId::new("policy2")]); - assert_eq!(response.residual_permits().collect::>(), vec![&PolicyId::new("policy3")]); - assert_eq!(response.true_forbids().collect::>(), vec![&PolicyId::new("policy4")]); - assert_eq!(response.false_forbids().collect::>(), vec![&PolicyId::new("policy5")]); - assert_eq!(response.error_forbids().collect::>(), vec![&PolicyId::new("policy6")]); - assert_eq!(response.residual_forbids().collect::>(), vec![&PolicyId::new("policy7")]); + assert_eq!( + response.reason().unwrap().collect::>(), + vec![&PolicyId::new("policy4")] + ); + + assert_eq!( + response.true_permits().collect::>(), + vec![&PolicyId::new("policy0")] + ); + assert_eq!( + response.false_permits().collect::>(), + vec![&PolicyId::new("policy1")] + ); + assert_eq!( + response.error_permits().collect::>(), + vec![&PolicyId::new("policy2")] + ); + assert_eq!( + response.residual_permits().collect::>(), + vec![&PolicyId::new("policy3")] + ); + assert_eq!( + response.true_forbids().collect::>(), + vec![&PolicyId::new("policy4")] + ); + assert_eq!( + response.false_forbids().collect::>(), + vec![&PolicyId::new("policy5")] + ); + assert_eq!( + response.error_forbids().collect::>(), + vec![&PolicyId::new("policy6")] + ); + assert_eq!( + response.residual_forbids().collect::>(), + vec![&PolicyId::new("policy7")] + ); } } From ce2ef49e8fa0379bc475e255b52c69de4c4335fa Mon Sep 17 00:00:00 2001 From: John Kastner <130772734+john-h-kastner-aws@users.noreply.github.com> Date: Wed, 17 Jun 2026 09:26:14 -0400 Subject: [PATCH 9/9] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- cedar-policy/CHANGELOG.md | 2 +- cedar-policy/src/api/tpe.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cedar-policy/CHANGELOG.md b/cedar-policy/CHANGELOG.md index 536c3eaf29..9e322764ee 100644 --- a/cedar-policy/CHANGELOG.md +++ b/cedar-policy/CHANGELOG.md @@ -14,7 +14,7 @@ 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 the determining policies on a TPE response (`TpeResponse::reason`, `TpeResponse::true_permits`, and `TpeResponse::true_forbids`). +- Functions for inspecting TPE policy evaluation results (`TpeResponse::reason` and iterators for true/false/error/residual permit/forbid policy IDs). ### Changed diff --git a/cedar-policy/src/api/tpe.rs b/cedar-policy/src/api/tpe.rs index 4ca1eaa940..f91906edb8 100644 --- a/cedar-policy/src/api/tpe.rs +++ b/cedar-policy/src/api/tpe.rs @@ -390,8 +390,8 @@ impl TpeResponse<'_> { /// after calling [`TpeResponse::reauthorize`] with a concrete request and entities. /// /// When [`TpeResponse::decision`] returns a concrete allow or deny, the - /// determining policies returned by this function are the satisfied permits - /// or forbids respectively. + /// determining policies returned by this function are exactly the policies from + /// [`TpeResponse::true_permits`] or [`TpeResponse::true_forbids`] respectively. /// /// If partial authorization does not reach a decision, then this function /// returns `None`. It's reasonable to treat this response as "no known