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
12 changes: 6 additions & 6 deletions cedar-policy-core/src/tpe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PolicyID> = 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<PolicyID> = 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<PolicyID> = 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<PolicyID> = 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<PolicyID> = 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<PolicyID> = residuals
let non_trivial_forbids: HashSet<&PolicyID> = residuals
.residual_forbids()
.map(|p| p.get_policy_id())
.collect();
Expand Down
90 changes: 66 additions & 24 deletions cedar-policy-core/src/tpe/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Comment thread
john-h-kastner-aws marked this conversation as resolved.
}
Comment thread
john-h-kastner-aws marked this conversation as resolved.

/// All literal uids referenced by this residual
Expand All @@ -87,16 +87,20 @@ impl From<ResidualPolicy> for Policy {
pub struct Response<'a> {
decision: Option<Decision>,
residuals: HashMap<PolicyID, ResidualPolicy>,
// All of the [`Effect::Permit`] policies that were satisfied
satisfied_permits: HashSet<PolicyID>,
// All of the [`Effect::Permit`] policies that were not satisfied
// All of the [`Effect::Permit`] policies that were true
true_permits: HashSet<PolicyID>,
// All of the [`Effect::Permit`] policies that were false
false_permits: HashSet<PolicyID>,
// All of the [`Effect::Permit`] policies that errored
error_permits: HashSet<PolicyID>,
// All of the [`Effect::Permit`] policies that evaluated to a residual
residual_permits: HashSet<PolicyID>,
// All of the [`Effect::Forbid`] policies that were satisfied
satisfied_forbids: HashSet<PolicyID>,
// All of the [`Effect::Forbid`] policies that were not satisfied
// All of the [`Effect::Forbid`] policies that were true
true_forbids: HashSet<PolicyID>,
// All of the [`Effect::Forbid`] policies that were false
false_forbids: HashSet<PolicyID>,
// All of the [`Effect::Forbid`] policies that errored
error_forbids: HashSet<PolicyID>,
// All of the [`Effect::Forbid`] policies that evaluated to a residual
residual_forbids: HashSet<PolicyID>,
// request used for this partial evaluation
Expand All @@ -117,11 +121,13 @@ 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 error_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 error_forbids = HashSet::new();
let mut residual_forbids = HashSet::new();
for rp in residuals {
let r = rp.get_residual();
Expand All @@ -130,28 +136,32 @@ impl<'a> Response<'a> {
match rp.get_effect() {
Effect::Forbid => {
if r.is_true() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: feels like this could be a good candidate to have an enum / match statement for in the future.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could write this as a match on the residual, but I don't think I see how it would make the code any nicer

satisfied_forbids.insert(id);
} else if r.is_false() || r.is_error() {
false_forbids.insert(id);
true_forbids.insert(id.clone());
} 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);
residual_forbids.insert(id.clone());
}
}
Effect::Permit => {
if r.is_true() {
satisfied_permits.insert(id);
} else if r.is_false() || r.is_error() {
false_permits.insert(id);
true_permits.insert(id.clone());
} 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);
residual_permits.insert(id.clone());
}
}
}
}

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(),
) {
Expand All @@ -170,11 +180,13 @@ impl<'a> Response<'a> {
Self {
decision,
residuals: residual_map,
satisfied_permits,
true_permits,
false_permits,
error_permits,
residual_permits,
satisfied_forbids,
true_forbids,
false_forbids,
error_forbids,
residual_forbids,
request,
entities,
Expand All @@ -188,7 +200,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())
}
Expand All @@ -199,7 +211,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())
}
Expand All @@ -215,6 +227,17 @@ impl<'a> Response<'a> {
.map(|id| self.residuals.get(id).unwrap())
}

/// Get trivially erroring permit residual policies

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Get trivially erroring permit residual policies
/// Get permit policies that errored during evaluation

nit: not sure I understand the word trivially in this context, maybe just drop it?

pub fn error_permits(&self) -> impl Iterator<Item = &ResidualPolicy> {
#[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<Item = &ResidualPolicy> {
#[expect(
Expand All @@ -226,6 +249,17 @@ impl<'a> Response<'a> {
.map(|id| self.residuals.get(id).unwrap())
}

/// Get trivially erroring forbid residual policies

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Get trivially erroring forbid residual policies
/// Get forbid policies that errored during evaluation

pub fn error_forbids(&self) -> impl Iterator<Item = &ResidualPolicy> {
#[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<Item = &ResidualPolicy> {
#[expect(
Expand Down Expand Up @@ -258,6 +292,14 @@ impl<'a> Response<'a> {
self.decision
}

/// Get the determining policies for the authorization decision
pub fn reason(&self) -> Option<impl Iterator<Item = &PolicyID>> {
match self.decision? {
Decision::Allow => Some(self.true_permits.iter()),
Decision::Deny => Some(self.true_forbids.iter()),
}
}

/// Perform reauthorization
pub fn reauthorize(
&self,
Expand Down
1 change: 1 addition & 0 deletions cedar-policy/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 TPE policy evaluation results (`TpeResponse::reason` and iterators for true/false/error/residual permit/forbid policy IDs).

### Changed

Expand Down
Loading
Loading